[Bug target/102485] -Wa,-many no longer has any effect

[amodra at gmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

--- Comment #10 from Alan Modra  ---
I have spent some time over the last few days digging into history relevant to
this bug as part of looking into a binutils report that an ARCH=powerpc
CROSS_COMPILE=powerpc-linux- pmac32_defconfig linux kernel no longer builds
with binutils-2.38.

As a result I've partly reverted the effect of binutils commit "ignore sticky
options for .machine", on both mainline binutils and the 2.38 branch.  Given
the way gcc operates, that commit was a mistake on my part.  The  initial
.machine passed by gcc will again keep -many, -maltivec, -mvsx, -mvle, -mspe or
-mspe2 in effect.  A .machine later in an assembly file, after some output to
any section (even .align counts), will be more strictly enforced.

That's the best I can do to solve this mess.  It does not completely fix this
bug, which I believe is a valid complaint.  For instance a user who has power10
asm() but does not want gcc generated power10 code, perhaps due to a gcc code
gen bug, might like to use -mcpu=power9 -Wa,-mpower10 on the gcc command line. 
Instead they would be forced to write ".machine power10" in their asm. 
(Ideally, .machine push; .machine power10; user asm; .machine pop, on each use
of power10 instructions.)  At least, that is the case for current mainline gcc
and binutils-2.38 where even "-mcpu=power9 -Wa,-many" won't work.  We simply
cannot dictate to users that their assembly needs rewriting.

A lot of this came about by accident.  On the gas side, it was by accident that
any of the gas command line cpu options were sticky for .machine.  The intent
behind the sticky options was to support people writing gas command lines like
"-maltivec -mppc" as well as the canonical "-mppc -maltivec".  When you
consider that gas also needs to support multiple cpu options on a command line,
with the last overriding any previous selection, it is more obvious why options
that add functional units like -maltivec are special.

On the gcc side, we have pr59828 and commit b9f12a01bedb5 which is where gcc
started to emit .machine rather than passing correct arguments to gas.  Current
mainline gcc continues to pass the wrong cpu to gas on the command line when
multiple -mcpu options are given to gcc, as can be seen by examining -v output
from the kernel compile mentioned above.  There have been multiple gcc bug
reports about that .machine directive, pr71216, pr91050, pr101393 to point out
some.  This means there are versions of gcc in widespread use that pass an
incorrect .machine to gas, which is why I say my gas change to make .machine
more strict was a mistake.  We can't expect the kernel and other users of the
toolchain to cope with gas using wrong cpu info if -many is disabled by
.machine.  Of course, the bugs in .machine might have been found earlier if
-many hadn't been covering them.  Similarly for bugs in the linux kernel
makefiles; mfsrin and mtsrin are not ppc64 instructions.

Overall, I think using .machine as a workaround for pr59828 was a mistake,
especially since digging into this bug reminded me that I'd submitted a
conceptually simple patch to fix pr59828 by passing on all the cpu args to gas.
 Which went unreviewed for a whole year before being dismissed.

[Bug target/102485] -Wa,-many no longer has any effect

[npiggin at gmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

--- Comment #9 from Nicholas Piggin  ---
And upstream gas still doesn't even warn with -many!!

[Bug target/102485] -Wa,-many no longer has any effect

[npiggin at gmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

--- Comment #8 from Nicholas Piggin  ---
(In reply to Segher Boessenkool from comment #7)
> > GCC already passes -m to the assembler though.
> 
> That mostly is historic.

So? I was pointing out the compiler already tells the assembler what
instruction set to use without the .machine directive.

> 
> > The justification for emitting the .machine directive is given as fixing a
> > build breakage due to a build system that passes an incorrect -m to the
> > assembler.
> 
> Not really, no. 

That is really the justification for emitting the .machine directive as
provided in the changelog of the commit which introduced the change.

> That is just one tiny part of the problem.  It is impossible
> to know what instruction sets we need ahead of time, and -many cannot work
> (and
> *does not* work: there are quite a few mnemonics that encode to different
> insns
> on different architecture versions (or for different CPUs), and we cannot
> know
> which is wanted, or which is preferred, ahead of time.

I understand the problems with -many, but it can and does work for real
software. E.g., Linux kernel as of before this change. It's not -many I'm
wedded to though, it's any ability to fix this sanely because of the .machine
directive.

The kernel should would change to using a specific CPU, e.g., -mcpu=power4
-Wa,-mpower10 and deal with the very rare few clashing mnemonics (e.g., tlbie)
specially with the .machine directive when an older one is required.

> 
> > *That* is the broken code (if any) that should have been fixed. But instead
> > that is hacked around in a way that breaks working code that passes down
> > -Wa,-many option as specified.
> 
> There is no working code that uses -many (accept by accident, if no problem
> has hit you yet).

I'll reword. "Instead that is hacked around in a way that breaks working code
that passes down the -Wa,-m option as specified."

> 
> > The kernel builds with a base compatibility (say -mcpu=power4) and then has
> > certain code paths that are dynamically taken if running on newer CPUs which
> > use newer instructions with inline asm.
> > 
> > This is an important usage and it's pervasive, it seems unreasonable to
> > break it.  Adding .machine directives throughout inline asm for every
> > instruction not in the base compatibility class is pretty horrible.
> 
> It is the only correct thing to do.

It's not. Not passing .machine and passing -mcpu is just as correct. With the
added bonus that it allows software to use a superset of instructions in such
cases. And even the great bonus that existing "broken" code that uses -many
will continue to work.

The correct way to deal with this is not to break this, it is to add a warning
to -many for some period to binutils to give code a chance to migrate. I'm all
for removing -many, and that is the right way to do it. By deprecating -many
and providing warnings. Not by hacking around it in the compiler that breaks
things.

[Bug target/102485] -Wa,-many no longer has any effect

[segher at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

Segher Boessenkool  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #7 from Segher Boessenkool  ---
> GCC already passes -m to the assembler though.

That mostly is historic.

> The justification for emitting the .machine directive is given as fixing a
> build breakage due to a build system that passes an incorrect -m to the
> assembler.

Not really, no.  That is just one tiny part of the problem.  It is impossible
to know what instruction sets we need ahead of time, and -many cannot work (and
*does not* work: there are quite a few mnemonics that encode to different insns
on different architecture versions (or for different CPUs), and we cannot know
which is wanted, or which is preferred, ahead of time.

> *That* is the broken code (if any) that should have been fixed. But instead
> that is hacked around in a way that breaks working code that passes down
> -Wa,-many option as specified.

There is no working code that uses -many (accept by accident, if no problem
has hit you yet).

> The kernel builds with a base compatibility (say -mcpu=power4) and then has
> certain code paths that are dynamically taken if running on newer CPUs which
> use newer instructions with inline asm.
> 
> This is an important usage and it's pervasive, it seems unreasonable to
> break it.  Adding .machine directives throughout inline asm for every
> instruction not in the base compatibility class is pretty horrible.

It is the only correct thing to do.

[Bug target/102485] -Wa,-many no longer has any effect

[npiggin at gmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

Nicholas Piggin  changed:

   What|Removed |Added

 CC||npiggin at gmail dot com

--- Comment #6 from Nicholas Piggin  ---
(In reply to Peter Bergner from comment #1)
> I agree it is GCC's job to emit a ".machine CPU" directive that allows the
> assembler to correctly assemble the code GCC generates.

GCC already passes -m to the assembler though.

The justification for emitting the .machine directive is given as fixing a
build breakage due to a build system that passes an incorrect -m to the
assembler.

*That* is the broken code (if any) that should have been fixed. But instead
that is hacked around in a way that breaks working code that passes down
-Wa,-many option as specified.

>  Your test case
> however uses inline asm and GCC does not know that you are using the mfppr32
> mnemonic.  The assembler code you write in an inline asm is basically a
> black box to the compiler.  It is therefor up to the programmer to ensure
> that either the -mcpu=CPU GCC option that is being used (which
> emits".machine CPU" directive) is enough to assemble the mnemonics in the
> inline asm or you have to emit them in your inline asm.

The kernel builds with a base compatibility (say -mcpu=power4) and then has
certain code paths that are dynamically taken if running on newer CPUs which
use newer instructions with inline asm.

This is an important usage and it's pervasive, it seems unreasonable to break
it.  Adding .machine directives throughout inline asm for every instruction not
in the base compatibility class is pretty horrible.

[Bug target/102485] -Wa,-many no longer has any effect

[segher at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

--- Comment #5 from Segher Boessenkool  ---
(In reply to Peter Bergner from comment #4)
> (In reply to Segher Boessenkool from comment #3)
> > (In reply to Paul Clarke from comment #2)
> > > "-many" is supposed to make those black boxes just work.  This worked 
> > > before
> > > recent changes to binutils/GCC.  Is there any valid use of "-Wa,-many" 
> > > now?
> > 
> > It will still work if you are "compiling" a .s or .S, no?
> 
> Correct, since GCC doesn't actually compile anything, the asm is just passed
> directly to the assembler as is (modulo preprocessing, but that won't emit a
> .machine directive).

Yes.  I just didn't verify it actually still works after the changes :-)

[Bug target/102485] -Wa,-many no longer has any effect

[bergner at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

--- Comment #4 from Peter Bergner  ---
(In reply to Segher Boessenkool from comment #3)
> (In reply to Paul Clarke from comment #2)
> > "-many" is supposed to make those black boxes just work.  This worked before
> > recent changes to binutils/GCC.  Is there any valid use of "-Wa,-many" now?
> 
> It will still work if you are "compiling" a .s or .S, no?

Correct, since GCC doesn't actually compile anything, the asm is just passed
directly to the assembler as is (modulo preprocessing, but that won't emit a
.machine directive).

[Bug target/102485] -Wa,-many no longer has any effect

[segher at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

--- Comment #3 from Segher Boessenkool  ---
(In reply to Paul Clarke from comment #2)
> GCC putting the base ".machine" directive at the beginning of the file makes
> any command-line use of "-many" (-Wa,-many) be ignored.  Is that OK? 

If people do not use -Wa,-many unnecessarily, it probably would be
good if the assembler somehow retained that option.  But I get the
impression that that option is used a lot for no good reason at all,
and that causes weird problems.

> "-many" is supposed to make those black boxes just work.  This worked before
> recent changes to binutils/GCC.  Is there any valid use of "-Wa,-many" now?

It will still work if you are "compiling" a .s or .S, no?

[Bug target/102485] -Wa,-many no longer has any effect

[pc at us dot ibm.com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

--- Comment #2 from Paul Clarke  ---
GCC putting the base ".machine" directive at the beginning of the file makes
any command-line use of "-many" (-Wa,-many) be ignored.  Is that OK?  "-many"
is supposed to make those black boxes just work.  This worked before recent
changes to binutils/GCC.  Is there any valid use of "-Wa,-many" now?

[Bug target/102485] -Wa,-many no longer has any effect

[bergner at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102485

Peter Bergner  changed:

   What|Removed |Added

 CC||amodra at gcc dot gnu.org,
   ||bergner at gcc dot gnu.org,
   ||dje at gcc dot gnu.org,
   ||meissner at gcc dot gnu.org,
   ||segher at gcc dot gnu.org,
   ||wschmidt at gcc dot gnu.org

--- Comment #1 from Peter Bergner  ---
I agree it is GCC's job to emit a ".machine CPU" directive that allows the
assembler to correctly assemble the code GCC generates.  Your test case however
uses inline asm and GCC does not know that you are using the mfppr32 mnemonic. 
The assembler code you write in an inline asm is basically a black box to the
compiler.  It is therefor up to the programmer to ensure that either the
-mcpu=CPU GCC option that is being used (which emits".machine CPU" directive)
is enough to assemble the mnemonics in the inline asm or you have to emit them
in your inline asm.  For the later, you can fix your test case like:

bergner@pike:~$ cat mfppr32.c 
long f () {
  long ppr;
  asm volatile ("mfppr32 %0" : "=r"(ppr));
  return ppr;
}
bergner@pike:~$ cat mfppr32-2.c 
long f () {
  long ppr;
  asm volatile (".machine push\n\
 .machine power7\n\
 mfppr32 %0\n\
 .machine pop" : "=r"(ppr));
  return ppr;
}
bergner@pike:~$ gcc -c mfppr32.c 
/tmp/ccSpp2V8.s: Assembler messages:
/tmp/ccSpp2V8.s:19: Error: unrecognized opcode: `mfppr32'
bergner@pike:~$ gcc -c mfppr32-2.c
bergner@pike:~$

Therefore, I'm not sure there is anything for GCC to do here.