Re: basic asm and memory clobbers - Proposed solution

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

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.