Bug 113652 - [14/15 regression] Failed bootstrap on ppc unrecognized opcode: `lfiwzx' with -mcpu=7450
Summary: [14/15 regression] Failed bootstrap on ppc unrecognized opcode: `lfiwzx' with...
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 14.0
: P2 normal
Target Milestone: 14.0
Assignee: Segher Boessenkool
URL:
Keywords: assemble-failure, build
Depends on:
Blocks:
 
Reported: 2024-01-29 13:01 UTC by Christopher Fore
Modified: 2024-04-26 10:56 UTC (History)
6 users (show)

See Also:
Host:
Target: powerpc
Build:
Known to work:
Known to fail:
Last reconfirmed: 2024-01-30 00:00:00


Attachments
original preprocessed file (44.07 KB, text/plain)
2024-01-29 13:01 UTC, Christopher Fore
Details
minimized preprocessed file (57 bytes, text/plain)
2024-01-29 17:51 UTC, Christopher Fore
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Fore 2024-01-29 13:01:46 UTC
Created attachment 57251 [details]
original preprocessed file

Steps to reproduce:
1. Attempt to build GCC 14 (latest snapshot attempted is Gentoo's 20240128)
2. Fails to assemble with:
/tmp/ccP8ev2f.s: Assembler messages:
/tmp/ccP8ev2f.s:85: Error: unrecognized opcode: `lfiwzx'

Originally reported downstream at: https://bugs.gentoo.org/921621

Command to reproduce:
gcc -mcpu=7450 -O1 -mvsx -c _kf_to_sd.i

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/powerpc-unknown-linux-gnu/14/lto-wrapper
Target: powerpc-unknown-linux-gnu
Configured with: /var/tmp/portage/sys-devel/gcc-14.0.0_pre20231119/work/gcc-14-20231119/configure --host=powerpc-unknown-linux-gnu --build=powerpc-unknown-linux-gnu --prefix=/usr --bindir=/usr/powerpc-unknown-linux-gnu/gcc-bin/14 --includedir=/usr/lib/gcc/powerpc-unknown-linux-gnu/14/include --datadir=/usr/share/gcc-data/powerpc-unknown-linux-gnu/14 --mandir=/usr/share/gcc-data/powerpc-unknown-linux-gnu/14/man --infodir=/usr/share/gcc-data/powerpc-unknown-linux-gnu/14/info --with-gxx-include-dir=/usr/lib/gcc/powerpc-unknown-linux-gnu/14/include/g++-v14 --disable-silent-rules --disable-dependency-tracking --with-python-dir=/share/gcc-data/powerpc-unknown-linux-gnu/14/python --enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt --disable-werror --with-system-zlib --enable-nls --without-included-gettext --disable-libunwind-exceptions --enable-checking=yes,extra --with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 14.0.0_pre20231119 p9' --with-gcc-major-version-only --enable-libstdcxx-time --enable-lto --disable-libstdcxx-pch --enable-shared --enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu --disable-multilib --disable-fixed-point --enable-targets=all --enable-libgomp --disable-libssp --disable-libada --disable-cet --disable-systemtap --disable-valgrind-annotations --disable-vtable-verify --disable-libvtv --without-zstd --without-isl --enable-default-pie --enable-host-pie --disable-host-bind-now --enable-default-ssp
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 14.0.0 20231119 (experimental) (Gentoo 14.0.0_pre20231119 p9)
Comment 1 Richard Biener 2024-01-29 13:07:57 UTC
What's the version of binutils you are using?
Comment 2 Christopher Fore 2024-01-29 13:12:48 UTC
I've tried with both 2.40 and 2.41
Comment 3 Erhard F. 2024-01-29 16:50:35 UTC
(In reply to Christopher Fore from comment #0)
> Created attachment 57251 [details]
> original preprocessed file
> 
> Steps to reproduce:
> 1. Attempt to build GCC 14 (latest snapshot attempted is Gentoo's 20240128)
> 2. Fails to assemble with:
> /tmp/ccP8ev2f.s: Assembler messages:
> /tmp/ccP8ev2f.s:85: Error: unrecognized opcode: `lfiwzx'
> 
> Originally reported downstream at: https://bugs.gentoo.org/921621
> 
> Command to reproduce:
> gcc -mcpu=7450 -O1 -mvsx -c _kf_to_sd.i
I did the Gentoo downstream bugreport where I tried to build GCC 14 with GCC 11 + binutils 2.41. Building was done on a 32 bit partition on my Talos II via 'setarch ppc32 emerge -1 gcc'.

As the PowerPC 7450 (aka PowerPC G4) is only capable of -maltivec but not -mvsx I only passed "-O2 -mcpu=7450 -mtune=7450" as CFLAGS.

Still my GCC 14 build spills this "unrecognized opcode: `lfiwzx'" which is a Power ISA 2.06 (POWER 7) opcode. GCC 11 builds fine on the same system.
Comment 4 Christopher Fore 2024-01-29 17:51:50 UTC
Created attachment 57255 [details]
minimized preprocessed file

Here's the minimized file (still errors)
Comment 5 Christopher Fore 2024-01-29 20:49:51 UTC
I've managed to get the args down to the following as well:

gcc -mcpu=7450 -O0 -ftree-coalesce-vars -mvsx -c _kf_to_sd.i

-mcpu=7450 seems to be (one of) the root causes along with -ftree-coalesce-vars. -mcpu=powerpc does not fail.
Comment 6 Kewen Lin 2024-01-30 03:05:37 UTC
I think this is related to r10-580-ge154242724b084 and this failure is expected and a use error.

With it applied, we don't always pass -many to assembler with CHECKING_P enabled. Actually compilers (gcc-13, gcc-12, gcc-11 or trunk) generate the same assembly, but because gcc-11/gcc-12/gcc-13 is built with --checking=release by default which doesn't set CHECKING_P while trunk is built with --checking=yes,extra by default which set CHECKING_P. So it causes the different behaviors so that further considered as regression unexpectedly.

The issue should be gone if trunk gets released as gcc-14 or it's built with --checking=release. IMO Alan's commit aims to help to expose more and more such unexpected use cases and users can fix them in place. As #c3 "PowerPC 7450 (aka PowerPC G4) is only capable of -maltivec but not -mvsx", so it's unexpected to have -mcpu=7450 meanwhile having -mvsx, could you check where the -mvsx comes from and fix it instead?  Thanks!

btw, a workaround option is to add -Wa,-many to restore the previous behavior that passing -many to assembler.
Comment 7 Kewen Lin 2024-01-30 03:26:52 UTC
oops, I meant --enable-checking rather than --checking.
Comment 8 Andrew Pinski 2024-01-30 03:44:51 UTC
So t-float128 has this line:
# Build the emulator without ISA 3.0 hardware support.
FP128_CFLAGS_SW          = -Wno-type-limits -mvsx -mfloat128 \
...

Which gets added to some of the libgcc object files while compiling:
$(fp128_softfp_obj)      : INTERNAL_CFLAGS += $(FP128_CFLAGS_SW)
$(fp128_ppc_obj)         : INTERNAL_CFLAGS += $(FP128_CFLAGS_SW)


The problem is CFLAGS gets added also. It seems like passing -mvsx enables some other instructions in GCC's code generation BUT does not enable it for the assembler ...
Comment 9 Kewen Lin 2024-01-30 06:16:29 UTC
(In reply to Andrew Pinski from comment #8)
> So t-float128 has this line:
> # Build the emulator without ISA 3.0 hardware support.
> FP128_CFLAGS_SW          = -Wno-type-limits -mvsx -mfloat128 \
> ...
> 
> Which gets added to some of the libgcc object files while compiling:
> $(fp128_softfp_obj)      : INTERNAL_CFLAGS += $(FP128_CFLAGS_SW)
> $(fp128_ppc_obj)         : INTERNAL_CFLAGS += $(FP128_CFLAGS_SW)
> 
> 
> The problem is CFLAGS gets added also. It seems like passing -mvsx enables
> some other instructions in GCC's code generation BUT does not enable it for
> the assembler ...

ah, just noticed that it's bootstrapping gcc. Stripping regression tag since I don't think it's actually a regression as comments above.

I found that the libgcc_cv_powerpc_float128 checking can pass with -mcpu=7450 -mabi=altivec -mvsx -mfloat128, the assembler options are "-a32 -mppc -mvsx -maltivec -mbig" is actually the same as what are used for the case #c5 compiling. So it looks that -mvsx is supposed to tell assembler to recognize vsx instructions but somehow "lfiwzx" is not counted as vsx instruction.

More specifically "xvadddp" is recognized by assembler with -mvsx while "lfiwzx" isn't.

$ cat t1.s
.machine "7450"
lfiwzx 1,0,9

$ cat t2.s
.machine "7450"
xvadddp 34,34,35

$ as -a32 -mppc -mvsx t1.s -o t1.o
t1.s: Assembler messages:
t1.s:2: Error: unrecognized opcode: `lfiwzx'
$ as -a32 -mppc -mvsx t2.s -o t2.o
$ echo $?
$ 0
Comment 10 Sam James 2024-01-30 06:18:31 UTC
(In reality, I think it is a regression, given:
a) it regresses non-release checking (which we sometimes use even for released versions, it's opt-in though);
b) it blocks further testing with GCC 14

but I understand the argument that if a release were made with it, it wouldn't be the end of the world by itself and it only affects a specific configuration.)
Comment 11 Kewen Lin 2024-01-30 06:37:25 UTC
In gcc, lfiwzx is guarded with TARGET_LFIWZX => TARGET_POPCNTD (ISA2.06), while -mvsx will guarantee TARGET_POPCNTD (ISA_2_6_MASKS_SERVER) set, so it considers lfiwzx is supported. IMHO the underlying philosophy is that having the capability of vsx the supported ISA level is at least 2.06, lfiwzx is supported from 2.06, so it's supported.

But binutils seems not to follow it:
{"xvadddp",     XX3(60,96),     XX3_MASK,    PPCVSX,    PPCVLE,         {XT6, XA6, XB6}},
{"lfiwzx",      X(31,887),      X_MASK,   POWER7|PPCA2, 0,              {FRT, RA0, RB}},
Both are guarded with different masks and apparently PPCVSX doesn't enable POWER7.

Hi Alan and Peter,

I wonder if assembler can enable POWER7 when PPCVSX gets enabled like what gcc adopts now?
Comment 12 Kewen Lin 2024-01-30 06:59:03 UTC
(In reply to Sam James from comment #10)
> (In reality, I think it is a regression, given:
> a) it regresses non-release checking (which we sometimes use even for
> released versions, it's opt-in though);

But I assumed that non-release checking on old releases should also fail, from non-release vs. non-release, the behavior doesn't change.

> b) it blocks further testing with GCC 14
> 

Sorry for that, put it back as you like. :)

> but I understand the argument that if a release were made with it, it
> wouldn't be the end of the world by itself and it only affects a specific
> configuration.)
Comment 13 Kewen Lin 2024-01-30 08:58:04 UTC
One more finding: without an explicit cpu type but -mvsx, gcc passes -mpower7 to assembler already, but if there is an explicitly specified cpu type, it won't do that. I think the reason why it doesn't always make it is that only the last cpu type wins and the passing can override some higher cpu type unexpectedly.

The fixing candidates seems to be:

diff --git a/libgcc/config/rs6000/t-float128 b/libgcc/config/rs6000/t-float128
index b09b5664af0..47b06d3c30d 100644
--- a/libgcc/config/rs6000/t-float128
+++ b/libgcc/config/rs6000/t-float128
@@ -74,7 +74,7 @@ fp128_includes         = $(srcdir)/soft-fp/double.h \
                           $(srcdir)/soft-fp/soft-fp.h

 # Build the emulator without ISA 3.0 hardware support.
-FP128_CFLAGS_SW          = -Wno-type-limits -mvsx -mfloat128 \
+FP128_CFLAGS_SW          = -Wno-type-limits -mvsx -mfloat128 -mcpu=power7 \
                            -mno-float128-hardware -mno-gnu-attribute \
                            -I$(srcdir)/soft-fp \
                            -I$(srcdir)/config/rs6000 \

Or

diff --git a/libgcc/config/rs6000/t-float128 b/libgcc/config/rs6000/t-float128
index b09b5664af0..bf4a5e6aaf0 100644
--- a/libgcc/config/rs6000/t-float128
+++ b/libgcc/config/rs6000/t-float128
@@ -74,7 +74,7 @@ fp128_includes         = $(srcdir)/soft-fp/double.h \
                           $(srcdir)/soft-fp/soft-fp.h

 # Build the emulator without ISA 3.0 hardware support.
-FP128_CFLAGS_SW          = -Wno-type-limits -mvsx -mfloat128 \
+FP128_CFLAGS_SW          = -Wno-type-limits -mvsx -mfloat128 -Wa,-many \
                            -mno-float128-hardware -mno-gnu-attribute \
                            -I$(srcdir)/soft-fp \
                            -I$(srcdir)/config/rs6000 \

As gcc considers -mvsx to imply -mcpu=power7 (appending onto the current specified cpu type if there is one) while assembler doesn't consider like that.
Comment 14 Alan Modra 2024-02-07 23:15:21 UTC
(In reply to Kewen Lin from comment #11)
> I wonder if assembler can enable POWER7 when PPCVSX gets enabled like what
> gcc adopts now?
That wouldn't be a solution, because gcc needs to deal with older assemblers that won't have such a change.  I don't have a strong opinion on whether newer versions of gas ought to enable all power7 instructions when given -mvsx, but I can say that -mvsx followed the precedent of other similar options, eg. -maltivec, -mlsp, where the option adds instructions for some function unit to the current cpu supported.
Comment 15 Peter Bergner 2024-02-09 04:21:38 UTC
(In reply to Kewen Lin from comment #11)
> In gcc, lfiwzx is guarded with TARGET_LFIWZX => TARGET_POPCNTD (ISA2.06),
> while -mvsx will guarantee TARGET_POPCNTD (ISA_2_6_MASKS_SERVER) set, so it
> considers lfiwzx is supported. IMHO the underlying philosophy is that having
> the capability of vsx the supported ISA level is at least 2.06, lfiwzx is
> supported from 2.06, so it's supported.
> 
> But binutils seems not to follow it:
> {"xvadddp",     XX3(60,96),     XX3_MASK,    PPCVSX,    PPCVLE,        
> {XT6, XA6, XB6}},
> {"lfiwzx",      X(31,887),      X_MASK,   POWER7|PPCA2, 0,             
> {FRT, RA0, RB}},
> Both are guarded with different masks and apparently PPCVSX doesn't enable
> POWER7.

That's because xvadddp is a VSX instruction (ie, mentioned in the VSX section of the ISA), while lfiwzx is a floating point instruction and part of the base ISA (for Power7 and above).  To me, that means the -mvsx assembler option is correct to not enable lfiwzx.  ...and as Alan mentioned, even changing the assembler to have -mvsx enable lfiwzx isn't a solution, since old already released assemblers would still be broken.

The problem seems to be that the GCC option -mvsx enables some base (ie, non-vsx) instructions not included in the 7450 which seems dangerous to me.  If the vsx support in the compiler really needs those base power7 instructions to function correctly, then we should be emitting an error when the user does -mcpu=CPU -mvsx and CPU is something less the power7.  If the vsx support doesn't really need those base power7 instructions to operate, then we shouldn't be enabling them.   

Mike, can you confirm whether our -mvsx VSX support requires those base power7 instructions or not?
Comment 16 Christopher Fore 2024-02-12 12:38:25 UTC
I tried to build older snapshots as well (back to 20231119) and it seems this assembler error still occurs back to there so I was wrong about this being a new thing.
Comment 17 Erhard F. 2024-02-15 10:29:40 UTC
Sidenote: Interestingly GCC 14 builds fine with -mcpu=970 -mtune=970 despite the PowerPC G5 (aka. 970, POWER4+) is before ISA 2.06 and not being able to use -mvsx either.

I used Gentoo snapshot gcc-14.0.1_pre20240211-r1.
Comment 18 Richard Biener 2024-03-27 13:46:28 UTC
Would be nice to fix but not a blocker since it will go away with release checking or persists on branches when checking is enabled.  P2.
Comment 19 Michael Meissner 2024-03-28 20:37:54 UTC
When I wrote the VSX support many years ago, I intended that -mvsx enable all of ISA 2.06, which includes ISA 2.05, etc.

My intentions were there 2 options for power7, one is the base ISA 2.07 support for everything except the VSX registers (i.e. -mpopcntd), and the other enables the floating point support.  The reason is the kernel needs to be built without floating point support.

If you say -mvsx, it should include the standard power7 integer instructions (-mpopcntd), power6 server instructions (i.e. -mhard-dfp, -mcmpb, -mrecip, -mpowerpc-gfxopt, and -mpowerpc-gpopt), etc.

VSX support assumes it can use lfiwax and lfiwzx.
Comment 20 Segher Boessenkool 2024-03-29 12:58:09 UTC
(In reply to Michael Meissner from comment #19)
> When I wrote the VSX support many years ago, I intended that -mvsx enable
> all of ISA 2.06

But that is not how we do things, and it can never work predictably and reliably.

If you want ISA 2.07 you have to use -mcpu=power8, or any other CPU that
implements that ISA level.  Using other options is counterintuitive, confusing,
and causes problems directly even.

> The reason is the kernel needs to
> be built without floating point support.

The kernel and many other things.  That's what -msoft-float, -mno-altivec, and
-mno-vsx are for.  Those options mean use no FPRs, no VRs, or no VSRs.  Nothing
more, nothing less.  

> If you say -mvsx, it should include the standard power7 integer instructions
> (-mpopcntd), power6 server instructions (i.e. -mhard-dfp, -mcmpb, -mrecip,
> -mpowerpc-gfxopt, and -mpowerpc-gpopt), etc.

No.  -mvsx means the compiler can use VSX things, is not prevented from it by
-mno-vsx.  There can be other reasons it can not use VSX, like, the targeted
CPU does not support VSX.

The option says absolutely nothing about any other instructions.

> VSX support assumes it can use lfiwax and lfiwzx.

Any CPU that supports VSX is ISA 2.07 at least, yes.
Comment 21 Segher Boessenkool 2024-03-29 13:04:40 UTC
(2.06, whoops)
Comment 22 Peter Bergner 2024-03-29 21:17:24 UTC
I kicked off a 7450 build on one of our BE system and I can confirm we hit this error.  I also saw us generating the 2 operand form of the mfcr instruction which also leads to an assembler error because the 7450 doesn't support that either.

Were we are in the build is compiling libgcc and the routines for handling KFmode values.  The build machinery is explicitly adding -mvsx -mfloat128 to the compiler options when building those source files and that seems bogus to me, since the 7450 does not have VSX hardware.  It's the explicit addition of the -mvsx option to the command line that is causing the lfiwzx and 2 operand mfcr instructions to be generated, not some internal mishandling of the -mcpu=7450 option.

My $0.02 worth is we should be generating an error when trying to use -msx with -mcpu=7450 or any other cpu that doesn't have VSX hardware. I also don't think we should be building these KFmode files which require VSX when the underlying cpu we're targeting doesn't support it.
Comment 23 Michael Meissner 2024-04-13 04:39:11 UTC
This is one of those things where there is no right answer in part because we need other things to flesh out the support.

The reason -mvsx was used is we need the VSX registers to build the IEEE 128-bit support in libgcc (KFmode values are passed in vector registers 32..63, i.e. the traditional Altivec registers).

In theory, we need only Altivec, but I felt at the time when I implemented this  that it opened up the door for lots of other things breaking due to the goofy nature of Altivec addresses omitting the bottom bits and no direct move support) and that VSX was the minimum ISA needed.

Now, from a practical matter, it should have been power8 as a minimum (due to direct move) but at the time I did the initial work, we were still actively supporting power7.

Then we have the issue that while the compiler can generate code on BE systems for IEEE 128-bit (either with software emulation or with the power9 hardware support) glibc only supports IEEE 128-bit on 64-bit LE.

So for a user it is useless to have IEEE 128-bit on BE systems.  But if somebody wanted to go through and do the work to enable the GLIBC support and other parts of the compiler/libraries that provide IEEE 128-bit.  But it is not a windmill I want to charge and tilt at.  But hey, if somebody wants to do the work to fix all of the support for this, go ahead.  I am not that person.

Note this is the classic catch 22 that we faced in the early days.  GCC has to support the stuff to a minimal amount even though users can't use it.  But you need that ability to generate the code to get glibc to do the support.

In terms of the immediate problem, you have several choices:

1) Ignore it and say to the users don't do that.

2) Prevent the IEEE 128-bit libgcc bits from being built on a BE or 32-bit LE system unless some configure switch is used.  Or just kick the can down the road, and don't provide a configure option in GCC 14, and if people are interested do it in GCC 15.

3) Only build the IEEE 128-bit libgcc bits if the user configured the compiler with --with-cpu=power7, --with-cpu=power8, --with-cpu=power9, --with-cpu=power10 (and in the future --with-cpu=power11 or --with-cpu=future).  This could be code that if __VSX__ is not defined, the libgcc support functions won't get built.  We would then remove the -mvsx option from the library support functions.

Though note, there is an issue in that if you don't use a --with-cpu= configure option, it won't build the bits.  Thus for the brave person trying to enable IEEE 128-bit for BE, they would have to configure with one of the IBM server platforms, while the majority of users would be using the old Apple boards, embedded platforms, or even AIX, etc.

Note, I will be on vacation from April 16th through the 23rd, and I probably won't bring a work laptop, which will mean I won't be able to answer email in that period.