Re: basic asm and memory clobbers

[David Wohlferd <dw at LimeGreenSocks dot com>]

On 11/23/2015 2:04 AM, Andrew Haley wrote:
> On 21/11/15 12:56, David Wohlferd wrote:
> > So, what now?
> >
> > While I'd like to take the big step and start kicking out warnings for
> > non-top-level right now, that may be too bold for phase 3.  A more
> > modest step for v6 would just provide a way to find them (maybe
> > something like -Wnon-top-basic-asm or -Wonly-top-basic-asm) and doc the
> > current behavior as well as the upcoming change.
> Warnings would be good.

Richard's suggestion was:

> I'm suggesting that we don't accept [basic asm] at all inside a 
function.  One must audit the source and make a conscious decision to 
write asm("bla" : ); instead.  Accepting basic asm outside of a function 
is perfectly ok.

I'm really not a compiler-writer, but I've taken a shot at implementing 
this.  While Richard is talking about completely deprecating this 
feature (a direction I support), I've started by emitting warnings, and 
by having the warnings disabled by default. This allows people to 
experiment with the new direction without getting clobbered by it.  My 
intent is something like this:

-Wonly-top-basic-asm
Warn if basic @code{asm} statements are used inside a function (ie not
at file scope/top level).  Due to the potential for unsafe optimizations,
always use extended instead of basic asm inside functions.  This
warning is disabled by default and is not enabled by -Wall or -Wextra.

I probably won't include the bits about Wall or Wextra in the actual doc 
patch.  They're here more to provoke comments in case someone thinks 
this behavior should change.  I'm open to suggestions about alternate 
names, too.

I've got this working for both c and c++.  It doesn't affect other 
places that use "asm" like "explicit register variables," "asm labels," 
"extended asm" or "top level basic asm."  It is also pleasantly small.  
As written, it should be useful to find places in current code that are 
at risk.

However (there's always a 'however'), it doesn't correctly handle 
"naked" functions (ie __attribute__((naked)) ).  By definition, naked 
functions can *only* include basic asm 
(https://gcc.gnu.org/ml/gcc/2014-05/msg00172.html).  So generating a 
warning for them is incorrect.

I'll need help fixing that.

I don't know if it is possible from within the parsers (where my current 
code is being added) to walk back up and get the attributes for the 
function.  I assume not.  In that case, I'll need some help finding some 
place up the call stack where you can.  Suggestions welcome.

The patch is at http://www.LimeGreenSocks.com/gcc/24414f.zip and 
includes test code.

> My warning still holds: there are modes of compilation on some
> machines where you can't clobber all registers without causing reload
> failures.  This is why Jeff didn't fix this in 1999.  So, if we really
> do want to clobber "all" registers in basic asm it'll take a lot of
> work.

I was always reluctant to see this change made.  In addition to the 
issues you mention, I had questions about the impact on the surrounding 
code.  I like Richard's direction much better.  We can start with a 
disabled warning, then upgrade as seems warranted.

dw