ping, CCing middle-end maintainers for review.
On 31 May 2013 10:13, Chung-Lin Tang <clt...@codesourcery.com> wrote: > On 13/5/15 8:12 PM, Richard Sandiford wrote: >> 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. > > Well I can't help those targets then, but at least nothing will be > changed for them by this patch. It will just continue to return 'true'. > >> 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. :-) > > Like we discussed below, I think the direction should be towards making > things more machine-independent, rather then pushing more into the backend. > >>> 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. > > > FWIW, I've ran tests on the newer patch on i686-linux, with no > regressions. Testcase has been moved to gcc.dg/torture by recommendation > of Bernhard. If any of the RTL maintainers can give an eye of merciful > approval, this old PR could be resolved :) > > Thanks, > Chung-Lin >
