| Message ID | 51935F65.4000803@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
Chung-Lin Tang <cltang@codesourcery.com> writes: > On 13/5/10 6:37 PM, Richard Sandiford wrote: >> Chung-Lin Tang <cltang@codesourcery.com> writes: >>> + case UNSPEC: >>> + /* Reach for a contained symbol. */ >>> + return nonzero_address_p (XVECEXP (x, 0, 0)); >> >> I don't think this is safe. UNSPECs really are unspecified :-), >> so we can't assume that (unspec X) is nonzero simply because X is. > > Attached is a modified patch (not yet tested but just for demonstration) > with a more specific test, hopefully regarded as more safe. > > The point is in recognizing (const (unspec [symbol] XYZ)) offsets in PIC > references, which seems quite idiomatic across all targets by now. I agree this is safer. However, there used to be some ports that use (plus pic_offset_table_rtx (symbol_ref X)) -- i.e. without an unspec -- to represent a PIC reference to X. That always seemed semantically wrong, since you're not actually adding the address of X and the PIC register, but the patch wouldn't handle that case correctly. An alternative might be to remove the pic_offset_table_rtx case altogether and rely on targetm.delegitimize_address instead. FWIW, I'd prefer that if it works, but it's not me you need to convince. :-) > I would suggest that this probably means there should be a new, more > specific construct in RTL to represent relocation values of this kind, > instead of (const (unspec)) serving an unofficial role; possibly some > real support for reasoning about PIC references could also be considered. Yeah, maybe we could try to introduce some target-independent knowledge of certain reloc types, a bit like the generic BFD_RELOC_*s in bfd. Thanks, Richard
Index: rtlanal.c =================================================================== --- rtlanal.c (revision 198923) +++ rtlanal.c (working copy) @@ -393,7 +393,15 @@ nonzero_address_p (const_rtx x) /* Handle PIC references. */ if (XEXP (x, 0) == pic_offset_table_rtx && CONSTANT_P (XEXP (x, 1))) - return true; + { + rtx offset = XEXP (x, 1); + if (GET_CODE (offset) == CONST + && GET_CODE (XEXP (offset, 0)) == UNSPEC + && GET_CODE (XVECEXP (XEXP (offset, 0), 0, 0)) == SYMBOL_REF) + return nonzero_address_p (XVECEXP (XEXP (offset, 0), 0, 0)); + + return true; + } return false; case PRE_MODIFY: