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

Reply via email to