This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: basic asm and memory clobbers - Proposed solution


On 12/1/2015 7:41 PM, Jeff Law wrote:
> My strong preference is still to document the desired semantics for GCC and treat anything that does not adhere to those semantics as a bug.

Despite nearly 100 posts over 2 threads, we don't seem to be reaching either a consensus or a conclusion. How do we move this from discussion to decision?

If the decision were mine to make, I'd just deprecate "basic asm in a function" and be done with it. But it's not. I don't know how the GCC project makes its decisions on issues like this, but the closest we've gotten is a "strong preference" for an approach with which I disagree.

I'm going to try to sum up what we've discussed, along with my take on the pluses and minuses of each proposed solution. If after reading this the global reviewers make a final decision (even one I disagree with), I'll try to help move it forward. Otherwise, I guess the discussion fades away and 24414 just sits for another 10 years.

-----------------------------------------
There are three problems related to basic asm that we are trying to solve:

#1 The existing (and historical) docs don't describe basic asm's behavior regarding clobbers (registers, memory, etc). #2 People have written basic asm code based on incorrect assumptions about its behavior (easy to do given #1). #3 Because basic asm has no dependencies (except 'volatile'), improvements to optimizers can move it in unexpected ways, sometimes breaking existing projects.

-----------------------------------------
And here are the three solutions that have been proposed:

Solution 1:
Just document the current behavior of basic asm.

People who have written incorrect code can be pointed at the text and told to fix their code. People whose code is damaged by optimizations can rewrite using extended to add dependencies (possibly doc this?).

Solution 2:
Change the docs to say that basic asm clobbers everything (memory, all registers, etc) or perhaps just memory (some debate here), but that due to bugs that will eventually be addressed, it doesn't currently work this way. Eventually (presumably in a future phase 1) modify the code to implement this.

People who have written their code incorrectly may have some hard-to-find problems solved for them. This is particularly valuable for projects that are no longer being maintained. And while clobbers aren't the best solution to dependencies, they can help.

Solution 3:
Deprecate (and eventually disallow) the use of basic asm within functions (perhaps excluding asm("") and naked functions). Do this by emitting warnings (and eventually fatal errors). Doc this plan.

Unless GCC adds a built-in assember, basic asm is always going to be a problem. The whole point of extended asm is to provide the information GCC needs to properly interface C code with asm. Warnings can point out where people have failed to provide that information, allowing them to correct it.

------------------------------------------------------------
Here's my take on the pros/cons of each:

Solution 1: Document the current behavior of basic asm.

Pro:
- Simplest to implement.
- Most backward compatible.

Con:
- Doesn't solve the problems for people who wrote their code wrong, except to let them know that they have. - Doesn't help with dependencies. Mentioning the problem and pointing them to extended might help. A bit.
- GCC will need to continue to tweak optimizers to work around problems.


Solution 2: Change basic to clobber memory and/or everything.

Pro:
- Docing the current and intended behavior lets people know what to expect going forward, even though we aren't prepared to implement the change in phase 3. - When the code change is checked in, long-time problems (of a kind that are REALLY hard to find) may be fixed.
- Adding a memory clobber (or clobber all) helps with dependencies. A bit.

Con:
- We haven't agreed exactly what the implementation details are. Docing them before writing the implementation is risky.
- Docing "someday" fixes is risky in general since someday may never come.
- Implementation may be harder than it sounds (for example how to handle frame pointers). - New users of basic asm won't be able to depend on any specific behavior (since we're explicitly saying the behavior will change). To be sure they'll always get the behavior they want, they'll have to use extended. - Existing users who realize they should have used the memory clobber won't want to wait for a future version of the compiler to fix this. They'll have to use extended too. - Existing users who want to future-proof their code to avoid having clobbers thrust upon them will also have to use extended asm. - While changing the "clobber nothing" semantics to "clobber everything" is WAY safer than doing the reverse, it's still not 100% safe. By definition basic asm is a fragile area. Any changes can conceivably result in failures. - Even without failures, adding memory/register clobbers to arbitrary places in people's code can have performance implications. - Given how many people "know" how this works (and have used that knowledge in their code, docs, blogs, books, posts, etc), changing this will cause confusion.


Solution 3: Deprecate/disallow the use of basic asm within functions.

Pro:
- By adding warnings today, people can find problems they already have in their code. - Migrating to extended asm allows users to clobber only what they need (which may be nothing). This produces more efficient code than forcing a clobber everything. - Migrating to extended asm allows users to add dependencies (via inputs/outputs) to prevent their code from being mis-optimized.
- Does not change the semantics of this very old instruction.
- Allows users to decide for themselves which is safer: Always clobber vs do "what it used to do." - Emitting warnings allows GCC to highlight source code that is affected rather than requiring people to scan their source trees for suspect code.

Con:
- (Eventually) requires a code change to existing code that uses "basic asm in a function." This is a big one. Partially offset by the fact that it's not a very difficult change once the statements are pointed out. And hopefully this usage is relatively rare. - It only solves the 'dependency' and 'incorrect assumption' problems if users fix them themselves when converting. Partially offset by creating a "How to convert basic asm to extended asm" doc that explains the issues.


-----------------------------------------
Status:
I have posted preliminary patches for two of these solutions:

Solution 1: https://gcc.gnu.org/ml/gcc/2015-11/msg00036.html
Solution 3: https://gcc.gnu.org/ml/gcc/2015-11/msg00198.html

Each of these would need a bit more work before being ready to post to patches, but until someone makes a final decision, it doesn't seem like a good use of my time to do more work on them.

dw


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]