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
