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
Index: rtlanal.c
===================================================================
--- rtlanal.c (revision 199474)
+++ 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:
Index: testsuite/gcc.dg/torture/pr32219.c
===================================================================
--- testsuite/gcc.dg/torture/pr32219.c (revision 0)
+++ testsuite/gcc.dg/torture/pr32219.c (revision 0)
@@ -0,0 +1,12 @@
+/* PR target/32219 */
+/* { dg-do run } */
+/* { dg-require-visibility "" } */
+/* { dg-options "-fPIC" { target fpic } } */
+
+extern void f() __attribute__((weak,visibility("hidden")));
+int main()
+{
+ if (f)
+ f();
+ return 0;
+}
