Re: basic asm and memory clobbers - Proposed solution
Hi,
On 12.12.2015 10:51, Andrew Haley wrote:
> On 11/12/15 22:18, David Wohlferd wrote:
>
>> 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.
> It's not just bugs which make clobbering all registers unwise and/or
> impossible.
>
>> 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.
> You've missed the most practical solution, which meets most common
> usage: clobber memory, but not registers. That allows most of the
> effects that people intuitively want and expect, but avoids the
> breakage of register clobbers. It allows basic asm to be used in a
> sensible way by pushing and popping all used registers.
>
> Andrew.
Yes, implicitly clobber memory and cc-flags, on targets where that is
also automatically added to the clobber list, even when the user does
not mention that in the extended asm syntax.
That is also my preferred solution, I prepared a patch for next stage1,
the details are still under discussion, but it is certainly do-able with
not too much effort.
See https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00938.html
The rationale for this is: there are lots of ways to write basic asm
statements, that may appear to work, if they clobber memory. Even if
they clobber CC that will also work most of the time.
And because you can use all global values simply by name, users will
expect that to be supported.
And, basic asm is a instruction scheduling barrier, that is similar to a
memory barrier. See sched-deps.c:
case ASM_INPUT:
{
/* Traditional and volatile asm instructions must be considered
to use
and clobber all hard registers, all pseudo-registers and all of
memory. So must TRAP_IF and UNSPEC_VOLATILE operations.
Consider for instance a volatile asm that changes the fpu
rounding
mode. An insn should not be moved across this even if it
only uses
pseudo-regs because it might give an incorrectly rounded
result. */
I think, this comment was written by Jeff Law in 1997, and in simple
cases (w/o loops and redundant assignments) the code below does 99%
exactly what the user expects. The problematic optimizations only
happen because of we did not have the implicit memory barriers yet,
which adds only the missing 1%.
My personal gut feeling on that warning is a bit mixed...
If we have a -Wall-enabled warning on asm("..."), people who know next
to nothing about assembler will be encouraged to "fix" this warning in a
part of the code which they probably do not understand at all. This
frightens me a bit.
Because I know they will soon find out, that adding a few colons fixes
the warning, but asm("...":::) is just more ugly, and will not be any
better IMHO.
For me, it is just very very unlikely that any piece of assembler really
clobbers nothing and has no inputs and no outputs at the same time, even
it it looks so at first sight...
It is much more likely that someone forgot to fill in the clobber section.
So for me it would also be good to warn on asm("...":::) and require
that, if they want to fix this warning, they must at least write
something in the clobber section, like asm ("...":::"nothing"); that
would be a new clobber name which can only stand alone and, which can
get stripped after the warning processing took place in the FE.
If the warning would warn on basic asm and extended asm with zero-input,
zero-output and zero-clobbers, only then I would regard that an
improvement. And for those few examples where we really have invented
something that does not clobber anything, I would say then you should
also write "nothing" in the clobber section.
Bernd.
gcc-6-20151213 is now available
Snapshot gcc-6-20151213 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/6-20151213/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 6 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/trunk revision 231601 You'll find: gcc-6-20151213.tar.bz2 Complete GCC MD5=96d3a19862e7bbdf9bed3d4c7166f9f7 SHA1=2927846b1443a4d7051b6d6dd96e406df29393d7 Diffs from 6-20151206 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-6 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
Re: basic asm and memory clobbers - Proposed solution
Is there a decision maker still teetering on the edge of making a call here? Or have they all moved on and we are just talking among ourselves? I keep worrying that if I don't reply, someone will swoop in, read the last message in the thread, and charge off to make a changes based on that. So... On 12/13/2015 1:25 AM, Bernd Edlinger wrote: That is also my preferred solution, I'm really not sure what the point of doing the memory clobber is. Yes, I see that it is easier to code than "clobber everything." And yes, it might help with certain types of bugs for people who have or might someday misuse asm. But if we are trying to give users what they expect, shouldn't we be doing the "clobber everything" Jeff suggested (https://gcc.gnu.org/ml/gcc/2015-11/msg00081.html)? Surely that's the "safest" answer, and it's as likely to be what people expect as anything else. Adding the memory clobber will already break backward compatibility and risk breaking existing code. Why do all that for half a solution? Yes it's harder, but please don't tell me we plan to change things yet again some day to do the rest? > The rationale for this is: there are lots of ways to write basic asm > statements, that may appear to work, if they clobber memory. > because you can use all global values simply by name, users will > expect that to be supported. > people who know next > to nothing about assembler will be encouraged to "fix" this warning in a > part of the code which they probably do not understand at all. So in the end, we are proposing a potentially breaking change, not because we are violating a standard, not to give users what (some people think) they expect, and not even because someone has actually reported a problem. But because someone might use it wrong, and if we just point the statement out to them, they might not know how to correct it. That seems like a lot of risk for so very little gain. And if we do end up changing this, using basic asm will become more dangerous than ever. In addition to all the old problems (that will still exist in earlier versions), now its behavior VARIES between versions. Programmers who want specific behavior (rather than our assumption-du-jour) will be forced to CHANGE THEIR CODE to account for this variability. So my solution highlights at-risk statements that must be changed to use extended asm. Your solution is that people will need to realize for themselves that the statements must change to extended in order to work the same way in newer versions, and then must find them all for themselves. Further, if people (correctly) fix the warnings I'm proposing, then they get correct behavior for all supported versions of the compiler. If people do the nothing you are proposing, they may still have problems even after the fix (if they assume registers get clobbered) or may have new problems (performance or otherwise), and will still have problems using any version of the compiler earlier than the one expected to be released sometime late next year. I prepared a patch for next stage1, the details are still under discussion, but it is certainly do-able with not too much effort. See https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00938.html I was unaware of this conversion. Unfortunate that you didn't think to cc me on this, as there are several points I would have made regarding the proposed code and doc changes. But mostly, I would have said what Bernd Schmidt said: "I'm not sure there was any consensus in that other thread" and "I think it would be best if we could deprecate basic asms in functions, or at least warn about them in -Wall." I have been talking myself blue in the fingers on this topic since early November and I honestly don't know if I have convinced anyone of anything. At this point, I really don't know what more I can say. It's pretty much all in https://gcc.gnu.org/ml/gcc/2015-12/msg00092.html FWIW. dw
