public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/102485] New: -Wa,-many no longer has any effect
@ 2021-09-25 21:27 pc at us dot ibm.com
  2021-10-04 17:26 ` [Bug target/102485] " bergner at gcc dot gnu.org
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: pc at us dot ibm.com @ 2021-09-25 21:27 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 102485
           Summary: -Wa,-many no longer has any effect
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pc at us dot ibm.com
  Target Milestone: ---

The assembler option "-many" tells the assembler to support assembly of
instructions from any vintage of processor. This can be passed through the GCC
compiler using the command line option "-Wa,-many".

The "-many" functionality has been under attack of late. ;-)

It once was that GCC would always pass "-many" to the assembler.  This was
stopped with commit e154242724b084380e3221df7c08fcdbd8460674 "Don't pass -many
to the assembler".

A recent change to binutils, commit b25f942e18d6ecd7ec3e2d2e9930eb4f996c258a
"ignore sticky options for .machine" stopped preserving "sticky"
options across a base `.machine` directive. This change caused sequences like:
.machine altivec
.machine power5
...to disable AltiVec instructions afterward, because "power5" did not support
AltiVec, and "power5" is a base ".machine" directive.

A perhaps unintended consequence is that using GCC to pass "-many" to the
assembler (via "-Wa,-many") has no effect because GCC adds a base ".machine"
directive to every(?) assembler file given to the assembler, but only passes
"-many" (no ".machine" directives are added). The assember sees the "-many"
parameter, then sees the base ".machine" directive, and suppresses any impact
of the "-many" parameter.

-- mfppr32.c:
long f () {
  long ppr;
  asm volatile ("mfppr32 %0" : "=r"(ppr));
  return ppr;
}
--
$ gcc -c ./mfppr32.c
gcc -c ./mfppr32.c
/tmp/ccAShoDb.s: Assembler messages:
/tmp/ccAShoDb.s:18: Error: unrecognized opcode: `mfppr32'
$ gcc -Wa,-many ./mfppr32.c
/tmp/cc0tRDPx.s: Assembler messages:
/tmp/cc0tRDPx.s:18: Error: unrecognized opcode: `mfppr32'
$ gcc -S -Wa,-many -O ./mfppr32.c
$ cat mfppr32.s
[edited for brevity]
        .file   "mfppr32.c"
        .machine ppc
        .section        ".text"
        .globl f
        .type   f, @function
f:
        mfppr32 3
        blr
$ as mfppr32.s
mfppr32.s: Assembler messages:
mfppr32.s:12: Error: unrecognized opcode: `mfppr32'

With older binutils, this worked:
$ older-as mfppr32.s
$
--

If binutils assembler (as) is doing the right thing now with respect to the
base ".machine" directives and sticky ".machine" directives, then it would
perhaps be GCCs responsibility to build an assembler file that allows for
passing the "-many" assembler command line option through GCC and have that
continue to work as likely expected.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
@ 2021-10-04 17:26 ` bergner at gcc dot gnu.org
  2021-10-04 18:17 ` pc at us dot ibm.com
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: bergner at gcc dot gnu.org @ 2021-10-04 17:26 UTC (permalink / raw)
  To: gcc-bugs

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

Peter Bergner <bergner at gcc dot gnu.org> 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 <bergner at gcc dot gnu.org> ---
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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
  2021-10-04 17:26 ` [Bug target/102485] " bergner at gcc dot gnu.org
@ 2021-10-04 18:17 ` pc at us dot ibm.com
  2021-10-05 22:11 ` segher at gcc dot gnu.org
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: pc at us dot ibm.com @ 2021-10-04 18:17 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Paul Clarke <pc at us dot ibm.com> ---
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?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
  2021-10-04 17:26 ` [Bug target/102485] " bergner at gcc dot gnu.org
  2021-10-04 18:17 ` pc at us dot ibm.com
@ 2021-10-05 22:11 ` segher at gcc dot gnu.org
  2021-10-05 22:58 ` bergner at gcc dot gnu.org
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: segher at gcc dot gnu.org @ 2021-10-05 22:11 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(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?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
                   ` (2 preceding siblings ...)
  2021-10-05 22:11 ` segher at gcc dot gnu.org
@ 2021-10-05 22:58 ` bergner at gcc dot gnu.org
  2021-10-05 23:41 ` segher at gcc dot gnu.org
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: bergner at gcc dot gnu.org @ 2021-10-05 22:58 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Peter Bergner <bergner at gcc dot gnu.org> ---
(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).

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
                   ` (3 preceding siblings ...)
  2021-10-05 22:58 ` bergner at gcc dot gnu.org
@ 2021-10-05 23:41 ` segher at gcc dot gnu.org
  2022-02-23 17:11 ` npiggin at gmail dot com
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: segher at gcc dot gnu.org @ 2021-10-05 23:41 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(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 :-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
                   ` (4 preceding siblings ...)
  2021-10-05 23:41 ` segher at gcc dot gnu.org
@ 2022-02-23 17:11 ` npiggin at gmail dot com
  2022-02-23 17:38 ` segher at gcc dot gnu.org
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: npiggin at gmail dot com @ 2022-02-23 17:11 UTC (permalink / raw)
  To: gcc-bugs

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

Nicholas Piggin <npiggin at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |npiggin at gmail dot com

--- Comment #6 from Nicholas Piggin <npiggin at gmail dot com> ---
(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<cpu> 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<cpu> 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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
                   ` (5 preceding siblings ...)
  2022-02-23 17:11 ` npiggin at gmail dot com
@ 2022-02-23 17:38 ` segher at gcc dot gnu.org
  2022-02-24  1:11 ` npiggin at gmail dot com
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: segher at gcc dot gnu.org @ 2022-02-23 17:38 UTC (permalink / raw)
  To: gcc-bugs

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

Segher Boessenkool <segher at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #7 from Segher Boessenkool <segher at gcc dot gnu.org> ---
> GCC already passes -m<cpu> 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<cpu> 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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
                   ` (6 preceding siblings ...)
  2022-02-23 17:38 ` segher at gcc dot gnu.org
@ 2022-02-24  1:11 ` npiggin at gmail dot com
  2022-02-24  1:25 ` npiggin at gmail dot com
  2022-02-24 11:07 ` amodra at gmail dot com
  9 siblings, 0 replies; 11+ messages in thread
From: npiggin at gmail dot com @ 2022-02-24  1:11 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Nicholas Piggin <npiggin at gmail dot com> ---
(In reply to Segher Boessenkool from comment #7)
> > GCC already passes -m<cpu> 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<cpu> 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<cpu> 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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
                   ` (7 preceding siblings ...)
  2022-02-24  1:11 ` npiggin at gmail dot com
@ 2022-02-24  1:25 ` npiggin at gmail dot com
  2022-02-24 11:07 ` amodra at gmail dot com
  9 siblings, 0 replies; 11+ messages in thread
From: npiggin at gmail dot com @ 2022-02-24  1:25 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Nicholas Piggin <npiggin at gmail dot com> ---
And upstream gas still doesn't even warn with -many!!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug target/102485] -Wa,-many no longer has any effect
  2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
                   ` (8 preceding siblings ...)
  2022-02-24  1:25 ` npiggin at gmail dot com
@ 2022-02-24 11:07 ` amodra at gmail dot com
  9 siblings, 0 replies; 11+ messages in thread
From: amodra at gmail dot com @ 2022-02-24 11:07 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Alan Modra <amodra at gmail dot com> ---
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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-02-24 11:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-25 21:27 [Bug target/102485] New: -Wa,-many no longer has any effect pc at us dot ibm.com
2021-10-04 17:26 ` [Bug target/102485] " bergner at gcc dot gnu.org
2021-10-04 18:17 ` pc at us dot ibm.com
2021-10-05 22:11 ` segher at gcc dot gnu.org
2021-10-05 22:58 ` bergner at gcc dot gnu.org
2021-10-05 23:41 ` segher at gcc dot gnu.org
2022-02-23 17:11 ` npiggin at gmail dot com
2022-02-23 17:38 ` segher at gcc dot gnu.org
2022-02-24  1:11 ` npiggin at gmail dot com
2022-02-24  1:25 ` npiggin at gmail dot com
2022-02-24 11:07 ` amodra at gmail dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).