Chung-Lin Tang <clt...@codesourcery.com> writes: > On 13/5/10 6:37 PM, Richard Sandiford wrote: >> Chung-Lin Tang <clt...@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