On powerpc-apple-darwin9 the test gcc.c-torture/compile/pr44707.c fails at any optimization level above -O0 for both -m32 and m64, probably since it has been committed: FAIL: gcc.c-torture/compile/pr44707.c -O1 (test for excess errors) FAIL: gcc.c-torture/compile/pr44707.c -O2 (test for excess errors) FAIL: gcc.c-torture/compile/pr44707.c -O3 -fomit-frame-pointer (test for excess errors) FAIL: gcc.c-torture/compile/pr44707.c -O3 -g (test for excess errors) FAIL: gcc.c-torture/compile/pr44707.c -Os (test for excess errors) FAIL: gcc.c-torture/compile/pr44707.c -O2 -flto (test for excess errors) FAIL: gcc.c-torture/compile/pr44707.c -O2 -fwhopr (test for excess errors) (see http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg00777.html ). The failure is the pervasive linker error on darwin: [karma] gcc/_gcc_clean% /opt/gcc/gcc4.6w/bin/gcc -c -O1 /opt/gcc/_gcc_clean/gcc/testsuite/gcc.c-torture/compile/pr44707.c /var/tmp//ccM4PMM6.s:15:non-relocatable subtraction expression, "_w" minus "L00000000001$pb" /var/tmp//ccM4PMM6.s:15:symbol: "_w" can't be undefined in a subtraction expression /var/tmp//ccM4PMM6.s:14:non-relocatable subtraction expression, "_w" minus "L00000000001$pb" /var/tmp//ccM4PMM6.s:14:symbol: "_w" can't be undefined in a subtraction expression /var/tmp//ccM4PMM6.s:13:non-relocatable subtraction expression, "_v" minus "L00000000001$pb" /var/tmp//ccM4PMM6.s:13:symbol: "_v" can't be undefined in a subtraction expression /var/tmp//ccM4PMM6.s:12:non-relocatable subtraction expression, "_v" minus "L00000000001$pb" /var/tmp//ccM4PMM6.s:12:symbol: "_v" can't be undefined in a subtraction expression in general due to missing or inconsistent symbols in the assembly.
confirmed for O1 : Apparently, we have concluded that the indirect references are not required when there is a 0 offset for the var. When the offset is non-zero (v.b, v.c, v.d), we correctly construct the indirect ref. for O0, all the references are correctly formed (and there is a corresponding L_w$non_lazy_ptr). --- .text .align 2 .globl _foo _foo: stw r31,-4(r1) ;, mflr r0 ;, bcl 20,31,L00000000001$pb ; L00000000001$pb: ; mflr r31 ;, mtlr r0 ;, addis r2,r31,ha16(_v-L00000000001$pb) ;,, la r10,lo16(_v-L00000000001$pb)(r2) ;,, addis r2,r31,ha16(_w-L00000000001$pb) ;,, la r11,lo16(_w-L00000000001$pb)(r2) ;,, addis r2,r31,ha16(L_v$non_lazy_ptr-L00000000001$pb) ;,, lwz r2,lo16(L_v$non_lazy_ptr-L00000000001$pb)(r2) ;,, addis r8,r31,ha16(L_v$non_lazy_ptr-L00000000001$pb) ;,, lwz r8,lo16(L_v$non_lazy_ptr-L00000000001$pb)(r8) ;,, addis r9,r31,ha16(L_v$non_lazy_ptr-L00000000001$pb) ;,, lwz r9,lo16(L_v$non_lazy_ptr-L00000000001$pb)(r9) ;,, ; 12 "/Volumes/ScratchCS/gcc-live-trunk/gcc/testsuite/gcc.c-torture/compile/pr44707.c" 1 /* 0(r10) 0(r11) 4(r2) 8(r8) 12(r9) */ ; v.a, w, v.b, v.c, v.d ; 0 "" 2 lwz r31,-4(r1) ;, blr ; .non_lazy_symbol_pointer L_v$non_lazy_ptr: .indirect_symbol _v .long 0 .subsections_via_symbols
Trying to simplify this, the following is enough to trigger the behavior: extern int w; void foo (void) { int e2 = w; __asm__ volatile ("/* %0 */" : : "ro" (e2)); } == (1) the code DTRT when the constraint is "r" or "o" ... but not when it is "ro". (2) essentially, up to combine, we have: (insn 5 2 6 2 (set:SI (reg:SI 122) (plus:SI (reg:SI 31 r31) (high:SI (const:SI (unspec:SI [ (symbol_ref:SI ("&L_w$non_lazy_ptr") [flags 0x400] <var_decl 0x420a3a20 w>) ] UNSPEC_MACHOPIC_OFFSET))))) ../tests/pr44707-x.c:7 96 {addsi3_high} (nil)) (insn 6 5 7 2 (set (reg/f:SI 121) (mem/u/c:SI (lo_sum:SI (reg:SI 122) (const:SI (unspec:SI [ (symbol_ref:SI ("&L_w$non_lazy_ptr") [flags 0x400] <var_decl 0x420a3a20 w>) ] UNSPEC_MACHOPIC_OFFSET))) [0 S4 A8])) ../tests/pr44707-x.c:7 348 {movsi_low} (expr_list:REG_DEAD (reg:SI 122) (expr_list:REG_EQUAL (symbol_ref:SI ("w") [flags 0x240] <var_decl 0x420a3a20 w>) (nil)))) (3) when "ro" is given, combine decides to delete insn 5 & 6 ... (4) ira decides to re-insert the load of a pointer to w .. but it doesn't (re-)insert the machopic indirect stuff... (note 5 2 6 2 NOTE_INSN_DELETED) (note 6 5 12 2 NOTE_INSN_DELETED) (insn 12 6 13 2 (set (reg:SI 2 r2) (plus:SI (reg:SI 31 r31) (high:SI (const:SI (unspec:SI [ (symbol_ref:SI ("w") [flags 0x240] <var_decl 0x420a3a20 w>) ] UNSPEC_MACHOPIC_OFFSET))))) ../tests/pr44707-x.c:7 96 {addsi3_high} (nil)) (insn 13 12 7 2 (set (reg:SI 2 r2) (lo_sum:SI (reg:SI 2 r2) (const:SI (unspec:SI [ (symbol_ref:SI ("w") [flags 0x240] <var_decl 0x420a3a20 w>) ] UNSPEC_MACHOPIC_OFFSET)))) ../tests/pr44707-x.c:7 14 {macho_low_si} (nil)) which leads to the assembler fail... ==== So ... I don't know whether the problem lies in combine (incorrectly eliminating the address load) -- or in ira (not re-introducing the indirect reference). Would welcome input from one of you gurus ... this is an irritating long-term fail (and, I guess, it might even have implications for other targets...)
Well, I guess the testcase is simply invalid for MachO, or the way MachO does this UNSPEC stuff is broken (not properly checked during legitimization) or not properly restoring the UNSPEC during reload (it's its duty, not IRAs).
(In reply to comment #3) Thanks.. > Well, I guess the testcase is simply invalid for MachO, well it works for -O0 (and for x86 darwin) ... so let's assume that MachO can do it ... > or the way MachO > does this UNSPEC stuff is broken (not properly checked during legitimization) OK, well I'll look at those (although I don't see any dependancy on optimization level in the legitimize code in rs6000.c or config/darwin.c). > or not properly restoring the UNSPEC during reload (it's its duty, not IRAs). I'll check this too.
(In reply to comment #4) > (In reply to comment #3) FWIW gcc-4.2.1 (Apple local) fails and clang passes the test (although clang's asm at version 2.9 is a bit long-winded c.f. GCC's ... but that's quite an old version). > > or not properly restoring the UNSPEC during reload (it's its duty, not IRAs). > > I'll check this too. Thanks Richi, this seems to be the problem - it appears that the reload legitimizer does not consider that the item might be 'undefined' (in the mach-o, local-to-the-file sense) - and, therefore, there was no mechanism to recreate the necessary indirection. so, this is a fix - which bootstraps and regtests: I'd very much like an opinion as to whether it's the _right_ fix ... this is an area with which I am not familiar and therefore could have missed some other guard that's needed. It's clearly a very infrequent (perhaps even non-existent) occurrence outside the particular asm test-case (which makes me wonder still if combine came to the right decision to remove the refs in the first place). Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 181027) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -6185,7 +6185,21 @@ rs6000_legitimize_reload_address (rtx x, enum mach #if TARGET_MACHO if (flag_pic) { - rtx offset = machopic_gen_offset (x); + rtx offset; + + /* Reload might present us with a case where there is a reference to + an undefined entity. */ + if (!machopic_symbol_defined_p (x) && !MACHO_DYNAMIC_NO_PIC_P) + { + offset = gen_rtx_SYMBOL_REF (Pmode, + machopic_indirection_name (x, + false)); + SYMBOL_REF_DATA (offset) = SYMBOL_REF_DATA (x); + machopic_define_symbol (gen_const_mem (Pmode,offset)); + x = offset; + } + + offset = machopic_gen_offset (x); x = gen_rtx_LO_SUM (GET_MODE (x), gen_rtx_PLUS (Pmode, pic_offset_table_rtx, gen_rtx_HIGH (Pmode, offset)), offset);
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > FWIW gcc-4.2.1 (Apple local) fails and clang passes the test (although clang's > asm at version 2.9 is a bit long-winded c.f. GCC's ... but that's quite an old > version). > > > > or not properly restoring the UNSPEC during reload (it's its duty, not IRAs). > > > > I'll check this too. > > Thanks Richi, this seems to be the problem - it appears that the reload > legitimizer does not consider that the item might be 'undefined' (in the > mach-o, local-to-the-file sense) - and, therefore, there was no mechanism to > recreate the necessary indirection. > > so, this is a fix - which bootstraps and regtests: > > I'd very much like an opinion as to whether it's the _right_ fix ... this is an > area with which I am not familiar and therefore could have missed some other > guard that's needed. > > It's clearly a very infrequent (perhaps even non-existent) occurrence outside > the particular asm test-case (which makes me wonder still if combine came to > the right decision to remove the refs in the first place). Combine also asks the target whether the result is valid, so yes. > Index: gcc/config/rs6000/rs6000.c > =================================================================== > --- gcc/config/rs6000/rs6000.c (revision 181027) > +++ gcc/config/rs6000/rs6000.c (working copy) > @@ -6185,7 +6185,21 @@ rs6000_legitimize_reload_address (rtx x, enum mach > #if TARGET_MACHO > if (flag_pic) > { > - rtx offset = machopic_gen_offset (x); > + rtx offset; > + > + /* Reload might present us with a case where there is a reference to > + an undefined entity. */ > + if (!machopic_symbol_defined_p (x) && !MACHO_DYNAMIC_NO_PIC_P) > + { > + offset = gen_rtx_SYMBOL_REF (Pmode, > + machopic_indirection_name (x, > + false)); > + SYMBOL_REF_DATA (offset) = SYMBOL_REF_DATA (x); > + machopic_define_symbol (gen_const_mem (Pmode,offset)); > + x = offset; > + } > + > + offset = machopic_gen_offset (x); > x = gen_rtx_LO_SUM (GET_MODE (x), > gen_rtx_PLUS (Pmode, pic_offset_table_rtx, > gen_rtx_HIGH (Pmode, offset)), offset); I suppose you should post this to gcc-patches@.
still not right .. generates wrong code for the first two accesses (missing the indirect load).
well, I was trying to be too complicated - we should just avoid trying to do the substitution unless we can see the var in the TU. When this is done: Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 181150) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -6169,6 +6169,7 @@ rs6000_legitimize_reload_address (rtx x, enum mach #if TARGET_MACHO && DEFAULT_ABI == ABI_DARWIN && (flag_pic || MACHO_DYNAMIC_NO_PIC_P) + && machopic_symbol_defined_p (x) #else && DEFAULT_ABI == ABI_V4 && !flag_pic .. the test-case generates beautiful (i.e. what one would have generated by hand) asm. reg-strapped on trunk (although there's a lot of noise on trunk right now because of recent changes) - doing 4.6 branch now. will post to patches if the 4.6 regstrap is also OK. ==== As an aside (non-Darwin comment - since powerpc-eabisim does exactly the same). It's not obvious why the perfectly sensible RTL pre-combine is thrown away and the process started over (I'm guessing it's because the asm insn has a zero computed cost). Effectively (to my untutored eye), we end up rebuilding what was there before combine from the reload ...
Ok. Yeah, combine has a habit of removing a complex thing at one point and rebuilding at another point, mainly to shorten the lifetime. Mentally, I guess I was expecting to see code motion to change lifetime.
The patch in comment #8 fixes this pr. I have only been able to regtest gcc and gfortran without regression. Thanks.
Author: iains Date: Sat Nov 12 14:12:26 2011 New Revision: 181315 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181315 Log: gcc: PR target/45233 * config/rs6000/rs6000.c (rs6000_legitimize_reload_address): Only expand a symbol ref. into an access when the entity is defined in the TU. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000.c
Author: iains Date: Sat Nov 12 14:14:43 2011 New Revision: 181316 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181316 Log: gcc: PR target/45233 * config/rs6000/rs6000.c (rs6000_legitimize_reload_address): Only expand a symbol ref. into an access when the entity is defined in the TU. Modified: branches/gcc-4_6-branch/gcc/ChangeLog branches/gcc-4_6-branch/gcc/config/rs6000/rs6000.c
fixed.