[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #22 from Jakub Kulik --- Eric and Rainer, thank you both very much for all that testing and the fix.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 Eric Botcazou changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #21 from Eric Botcazou --- Fixed on Solaris for GCC 14 and later.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #20 from GCC Commits --- The master branch has been updated by Eric Botcazou : https://gcc.gnu.org/g:1d238c84025aaef1641e4000bd2a8f4328b474dd commit r14-10119-g1d238c84025aaef1641e4000bd2a8f4328b474dd Author: Eric Botcazou Date: Thu Apr 25 12:44:14 2024 +0200 Fix calling convention incompatibility with vendor compiler For the 20th anniversary of https://gcc.gnu.org/gcc-3.4/sparc-abi.html, a new calling convention incompatibility with the vendor compiler (and the ABI) has been discovered in 64-bit mode, affecting small structures containing arrays of floating-point components. The decision has been made to fix it on Solaris only at this point. gcc/ PR target/114416 * config/sparc/sparc.h (SUN_V9_ABI_COMPATIBILITY): New macro. * config/sparc/sol2.h (SUN_V9_ABI_COMPATIBILITY): Redefine it. * config/sparc/sparc.cc (fp_type_for_abi): New predicate. (traverse_record_type): Use it to spot floating-point types. (compute_fp_layout): Also deal with array types. gcc/testsuite/ * gcc.target/sparc/small-struct-1.c: New test. * gcc.target/sparc/pr105573.c: Rename to... * gcc.target/sparc/20230425-1.c: ...this. * gcc.target/sparc/pr109541.c: Rename to... * gcc.target/sparc/20230607-1.c: ...this
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #19 from ro at CeBiTec dot Uni-Bielefeld.DE --- > --- Comment #18 from ro at CeBiTec dot Uni-Bielefeld.DE Uni-Bielefeld.DE> --- >> --- Comment #17 from Eric Botcazou --- [...] >>> The sparc64-unknown-linux-gnu one will be running for a couple more >>> hours, though. >> >> The change should be a no-op for this platform. > > True. However, I'd rather wait for the bootstrap to complete. Should > be sometime tonight... That's finished now, too, and as expected there were no regressions.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #18 from ro at CeBiTec dot Uni-Bielefeld.DE --- > --- Comment #17 from Eric Botcazou --- >> The sparc-sun-solaris2.11 bootstrap (both multilibs) has just completed >> successfully without regressions. >> >> However, sparc/sol2.h needed an #undef to fix [...] > Thanks, fixed. great, thanks. >> The sparc64-unknown-linux-gnu one will be running for a couple more >> hours, though. > > The change should be a no-op for this platform. True. However, I'd rather wait for the bootstrap to complete. Should be sometime tonight... >> Btw., I thought about running gcc.dg/compat against Studio 12.6 cc. I'd >> started trying some time ago, but got distracted. At the very least, I >> needed -features=extensions -DSKIP_COMPLEX_INT for a considerable part >> of that testsuite to compile at all, for the likes of >> >> "/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/compat/pr102024_test.h", >> line 7: zero-sized struct/union >> >> "/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/compat/struct-layout-1.h", >> line 197: invalid type combination > > I used to do that on a regular basis 20 years ago, which led to: > https://gcc.gnu.org/gcc-3.4/sparc-abi.html > but I lost access to Sun Studio at some point. FWIW, cfarm216, the new Solaris 11.4/SPARC compile farm host, does have Studio 12.6 installed. Right now, it's only the FCS version, but I hope to get access to the studiosupport repo in the future. Besides, I've checked the testcase that prompted this PR with previous Studio versions quite some time back and their behaviour in this regard is unchanged.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #17 from Eric Botcazou --- > The sparc-sun-solaris2.11 bootstrap (both multilibs) has just completed > successfully without regressions. > > However, sparc/sol2.h needed an #undef to fix > > In file included from ./tm.h:27, > from /vol/gcc/src/hg/master/local/gcc/gencheck.cc:23: > /vol/gcc/src/hg/master/local/gcc/config/sparc/sol2.h:460:9: error: > "SUN_V9_ABI_COMPATIBILITY" redefined [-Werror] > 460 | #define SUN_V9_ABI_COMPATIBILITY 1 > | ^~~~ > In file included from ./tm.h:24: > /vol/gcc/src/hg/master/local/gcc/config/sparc/sparc.h:1705:9: note: this is > the location of the previous definition > 1705 | #define SUN_V9_ABI_COMPATIBILITY 0 > | ^~~~ Thanks, fixed. > The sparc64-unknown-linux-gnu one will be running for a couple more > hours, though. The change should be a no-op for this platform. > Btw., I thought about running gcc.dg/compat against Studio 12.6 cc. I'd > started trying some time ago, but got distracted. At the very least, I > needed -features=extensions -DSKIP_COMPLEX_INT for a considerable part > of that testsuite to compile at all, for the likes of > > "/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/compat/pr102024_test.h", > line 7: zero-sized struct/union > > "/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/compat/struct-layout-1.h", > line 197: invalid type combination I used to do that on a regular basis 20 years ago, which led to: https://gcc.gnu.org/gcc-3.4/sparc-abi.html but I lost access to Sun Studio at some point.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #16 from ro at CeBiTec dot Uni-Bielefeld.DE --- > --- Comment #15 from ro at CeBiTec dot Uni-Bielefeld.DE Uni-Bielefeld.DE> --- >> --- Comment #14 from Eric Botcazou --- >> Do you happen to have some spare cycles to conduct a testing cycle of the >> above >> tentative fix? It only affects the 64-bit ABI so a sparc64/sparcv9 one would >> be sufficient (Unfortunately I no longer have access to my SPARC/Solaris >> setup >> and haven't tried the Compile Farm yet). > > Sure: I've kept one half of the T8-1 hosting the new Solaris 11.4/SPARC > cfarm node to myself ;-) I've also got a Linux/sparc64 LDom around > since the cfarm instance has been very unstable lately. The sparc-sun-solaris2.11 bootstrap (both multilibs) has just completed successfully without regressions. However, sparc/sol2.h needed an #undef to fix In file included from ./tm.h:27, from /vol/gcc/src/hg/master/local/gcc/gencheck.cc:23: /vol/gcc/src/hg/master/local/gcc/config/sparc/sol2.h:460:9: error: "SUN_V9_ABI_COMPATIBILITY" redefined [-Werror] 460 | #define SUN_V9_ABI_COMPATIBILITY 1 | ^~~~ In file included from ./tm.h:24: /vol/gcc/src/hg/master/local/gcc/config/sparc/sparc.h:1705:9: note: this is the location of the previous definition 1705 | #define SUN_V9_ABI_COMPATIBILITY 0 | ^~~~ The sparc64-unknown-linux-gnu one will be running for a couple more hours, though. Btw., I thought about running gcc.dg/compat against Studio 12.6 cc. I'd started trying some time ago, but got distracted. At the very least, I needed -features=extensions -DSKIP_COMPLEX_INT for a considerable part of that testsuite to compile at all, for the likes of "/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/compat/pr102024_test.h", line 7: zero-sized struct/union "/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/compat/struct-layout-1.h", line 197: invalid type combination
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #15 from ro at CeBiTec dot Uni-Bielefeld.DE --- > --- Comment #14 from Eric Botcazou --- > OK, thanks, let's go ahead for Solaris then, but I agree that we'd better do > nothing for other platforms at this point. Right, I forgot that there are others (and I cannot test any of them at this point; I don't even know if current BSDs still support SPARC at all). > Do you happen to have some spare cycles to conduct a testing cycle of the > above > tentative fix? It only affects the 64-bit ABI so a sparc64/sparcv9 one would > be sufficient (Unfortunately I no longer have access to my SPARC/Solaris setup > and haven't tried the Compile Farm yet). Sure: I've kept one half of the T8-1 hosting the new Solaris 11.4/SPARC cfarm node to myself ;-) I've also got a Linux/sparc64 LDom around since the cfarm instance has been very unstable lately.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #14 from Eric Botcazou --- OK, thanks, let's go ahead for Solaris then, but I agree that we'd better do nothing for other platforms at this point. Do you happen to have some spare cycles to conduct a testing cycle of the above tentative fix? It only affects the 64-bit ABI so a sparc64/sparcv9 one would be sufficient (Unfortunately I no longer have access to my SPARC/Solaris setup and haven't tried the Compile Farm yet).
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #13 from ro at CeBiTec dot Uni-Bielefeld.DE --- > --- Comment #12 from Eric Botcazou --- > Rainer, what's your take on this? Should we proceed and change the ABI on > Solaris for GCC 14? I think so, yes: * Binary compatibility with the Studio compilers is pretty important IMO. * Even though the SCD wording isn't really clear, the intent seems to match what Studio cc does according to the colleague on the spec group Jakub cited. * There's no point in waiting beyond GCC 14, I believe: this will break compatiblity with GCC <= 13 no matter what. * Besides, Solaris is pretty quick picking up new GCC releases these days, so they'll bundle GCC 14.1.0 not long after it's released, so the benefit will be immediate. * I'm a tad uncertain about what to do on Linux/sparc64, though: while it mostly has followed the SCD, there are exceptions (e.g. sizeof (long double)) and no vendor compiler to be compatible with. So making the same change there would only mean breaking compatibility with older GCCs (and clang) for little gain.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #12 from Eric Botcazou --- Rainer, what's your take on this? Should we proceed and change the ABI on Solaris for GCC 14?
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #11 from Jakub Kulik --- > This is a bit of circular reasoning but, if the rule had been crystal clear, > GCC would have implemented it at some point during the last quarter of > century. I see. I guess it's also not a common enough use case to pass small structs with float arrays between programs and libraries (potentially compiled with different compilers). That said, for example libffi implements the ABI as was intended (it's how I originally found this issue).
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #10 from Jakub Kulik --- Sorry for longer response. I asked internally again and was told by a colleague who was in the room when the spec was created, that: "the intent was (and is) that the individual elements/atoms/fundamental types that make up a small structure, no matter how those elements/atoms/fundamental types are aggregated within the structure, are passed in registers appropriate for the fundamental type in question. (That is, pointers and integral types are passed in the %o registers, and floating point types are passed in the floating-point registers.) So a structure that contains an array of two floats is treated the same as a structure that contains two floats." That said, he agreed that the spec could perhaps be better written. He also added: Page 3P-11 says this, under "Function Argument Passing": "It is convenient to describe parameter linkage in terms of an array. Conceptually, parameters are assigned into an array of extended words, left-to-right, with an occasional “hole” to satisfy alignment restrictions. Typically, most parameter values will be “promoted” from their memory locations into registers, and most calls are expected to execute this way with less overhead." There is then a diagram that shows the correspondence between parameter registers and the parameter array. On page 3P-12, under "Structure and Union arguments", it says this: "Structure or union types up to eight bytes in size are assigned to one parameter array word, and align to eight-byte boundaries. "Structure or union types larger than eight bytes, and up to sixteen bytes in size are assigned to two consecutive parameter array words, and align according to the alignment requirements of the structure or at least to an eight-byte boundary." So perhaps instead of saying "The individual fields of a structure ... are subject to promotion into registers based on their type using the same rules as apply to scalar values" the spec should have said "The individual parameter array words assigned to a structure ... are subject to promotion into registers based on their type using the same rules as apply to scalar values."
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #9 from Eric Botcazou --- > Thank you for the proposed fix! I tested it with several programs that I > used to find/reproduce the issue and it seems to work now (I talked about > this with Rainer initially). OK, thanks for the testing!
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #8 from Eric Botcazou --- > Hmm, I just realized that you referred to the same sections, so my previous > comment might not make it clearer... Yes, the fields in question have array types so the rules about scalar values do not obviously apply to them. This is a bit of circular reasoning but, if the rule had been crystal clear, GCC would have implemented it at some point during the last quarter of century. My interpretation is that the writers of the ABI document probably overlooked the specific cases of arrays, which cannot appear as types of standalone parameters but can as types of fields in structures.
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #7 from Jakub Kulik --- Hmm, I just realized that you referred to the same sections, so my previous comment might not make it clearer...
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 Jakub Kulik changed: What|Removed |Added CC||jakub.kulik at oracle dot com --- Comment #6 from Jakub Kulik --- Thank you for the proposed fix! I tested it with several programs that I used to find/reproduce the issue and it seems to work now (I talked about this with Rainer initially). As for the ABI being potentially unclear, I am in no way a SPARCv9 ABI expert, so I asked internally, and was told that the ABI should be clear about this case: """ See page 3P-10 (PDF page 46) where it says this: %f0,%f1,%f2,%f3 (%d0, %d2) (%q0) Floating-point return values appear in the floating-point registers. Single-precision values occupy %f0; double-precision values occupy %d0; quad-precision values occupy %q0. (Refer to the SPARCTM Architecture Manual, Version 9 for details on the register numbering scheme). Otherwise, these are scratch registers. and %f0 through %f7 (%d0 through %d6) (%q0 and %q4) Floating-point fields from structure return values with a total size of 32 bytes or less appear in the floating-point registers. Then on page 3P-13 (PDF page 49) it says this: Structure or Union return values Structure and union return types up to thirty-two bytes in size are returned in registers. The registers are assigned as if the value was being passed as the first argument to a function with a known prototype. So we have to refer back to "Structure and Union arguments" on page 3P-12 (PDF page 48) where it says: "Structure or union types are always left-justified, whether stored in registers or memory. *The individual fields of a structure (or containing storage unit in the case of bit fields) are subject to promotion into registers based on their type using the same rules as apply to scalar values* (with the addition that a single-precision floating-point number assigned to the left half of an argument slot will be promoted into the corresponding even-numbered float register.)." [sic; emphasis added.] """
[Bug target/114416] calling convention incompatibility with vendor compiler for V9
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114416 --- Comment #5 from Eric Botcazou --- Created attachment 57806 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57806=edit Tentative fix
