* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
@ 2017-04-23 18:33 Bernd Edlinger
2017-04-28 17:17 ` Jeff Law
0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2017-04-23 18:33 UTC (permalink / raw)
To: Jeff Law, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 395 bytes --]
Hi Jeff,
this patch tries to fix the handling of pic_offset_rtx +
const(unspec(symbol_ref)+ const_int) in may_trap_p,
and restores the original state of affairs in update_equiv_regs.
What do you think is it OK to extract the symbol_ref out
of the unspec in this way, or is does it need a target hook?
Patch works at least for x86_64 and arm.
Is it OK for trunk?
Bernd.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-pr79286.diff --]
[-- Type: text/x-patch; name="patch-pr79286.diff", Size: 2459 bytes --]
2017-04-23 Bernd Edlinger <bernd.edlinger@hotmail.de>
rtl-optimizatoin/79286
* ira.c (update_equiv_regs): Revert to using may_tap_p again.
* rtlanal.c (rtx_addr_can_trap_p_1): SYMBOL_REF_FUNCTION_P can never
trap. Extract constant offset from pic_offset_table_rtx +
const(unspec(symbol_ref)+int_val) and pic_offset_table_rtx +
const(unspec(symbol_ref)), otherwise RTL may trap.
Index: gcc/ira.c
===================================================================
--- gcc/ira.c (revision 247077)
+++ gcc/ira.c (working copy)
@@ -3551,7 +3551,8 @@ update_equiv_regs (void)
if (DF_REG_DEF_COUNT (regno) == 1
&& note
&& !rtx_varies_p (XEXP (note, 0), 0)
- && def_dominates_uses (regno))
+ && (!may_trap_or_fault_p (XEXP (note, 0))
+ || def_dominates_uses (regno)))
{
rtx note_value = XEXP (note, 0);
remove_note (insn, note);
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c (revision 247077)
+++ gcc/rtlanal.c (working copy)
@@ -485,7 +485,7 @@ rtx_addr_can_trap_p_1 (const_rtx x, HOST_WIDE_INT
case SYMBOL_REF:
if (SYMBOL_REF_WEAK (x))
return 1;
- if (!CONSTANT_POOL_ADDRESS_P (x))
+ if (!CONSTANT_POOL_ADDRESS_P (x) && !SYMBOL_REF_FUNCTION_P (x))
{
tree decl;
HOST_WIDE_INT decl_size;
@@ -645,8 +645,23 @@ rtx_addr_can_trap_p_1 (const_rtx x, HOST_WIDE_INT
case PLUS:
/* An address is assumed not to trap if:
- it is the pic register plus a constant. */
- if (XEXP (x, 0) == pic_offset_table_rtx && CONSTANT_P (XEXP (x, 1)))
- return 0;
+ if (XEXP (x, 0) == pic_offset_table_rtx
+ && GET_CODE (XEXP (x, 1)) == CONST)
+ {
+ x = XEXP (XEXP (x, 1), 0);
+ if (GET_CODE (x) == UNSPEC
+ && GET_CODE (XVECEXP (x, 0, 0)) == SYMBOL_REF)
+ return rtx_addr_can_trap_p_1(XVECEXP (x, 0, 0),
+ offset, size, mode, unaligned_mems);
+ if (GET_CODE (x) == PLUS
+ && GET_CODE (XEXP (x, 0)) == UNSPEC
+ && GET_CODE (XVECEXP (XEXP (x, 0), 0, 0)) == SYMBOL_REF
+ && CONST_INT_P (XEXP (x, 1)))
+ return rtx_addr_can_trap_p_1(XVECEXP (XEXP (x, 0), 0, 0),
+ offset + INTVAL (XEXP (x, 1)),
+ size, mode, unaligned_mems);
+ return 1;
+ }
/* - or it is an address that can't trap plus a constant integer. */
if (CONST_INT_P (XEXP (x, 1))
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-23 18:33 [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs Bernd Edlinger
@ 2017-04-28 17:17 ` Jeff Law
2017-04-28 18:05 ` Bernd Edlinger
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2017-04-28 17:17 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches
On 04/23/2017 05:54 AM, Bernd Edlinger wrote:
> Hi Jeff,
>
>
> this patch tries to fix the handling of pic_offset_rtx +
> const(unspec(symbol_ref)+ const_int) in may_trap_p,
> and restores the original state of affairs in update_equiv_regs.
>
>
> What do you think is it OK to extract the symbol_ref out
> of the unspec in this way, or is does it need a target hook?
>
>
> Patch works at least for x86_64 and arm.
> Is it OK for trunk?
>
>
> Bernd.
>
>
> patch-pr79286.diff
>
>
> 2017-04-23 Bernd Edlinger<bernd.edlinger@hotmail.de>
>
> rtl-optimizatoin/79286
> * ira.c (update_equiv_regs): Revert to using may_tap_p again.
> * rtlanal.c (rtx_addr_can_trap_p_1): SYMBOL_REF_FUNCTION_P can never
> trap. Extract constant offset from pic_offset_table_rtx +
> const(unspec(symbol_ref)+int_val) and pic_offset_table_rtx +
> const(unspec(symbol_ref)), otherwise RTL may trap.
ISTM that rtx_addr_can_trap_p_1 is fundamentally broken in that
references via pic_offset_table_rtx can certainly trap.
Whether or not a given reference traps is a function of the size of
table (not known until link time) and the CONST_INT within the address
expression. So I'd be more inclined to remove the special casing of PIC
addresses where entirely -- that seemed pretty risky during stage3, but
now would be an appropriate time to tackle that.
If we were to try and keep the special handling, you certainly can't
depend on the form looking like (const(unspec(symbol_ref) + const_int).
You could have high/lo_sums were and probably other forms too. We allow
the backends to largely define what PIC address looks like.
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-28 17:17 ` Jeff Law
@ 2017-04-28 18:05 ` Bernd Edlinger
2017-04-28 19:01 ` Jeff Law
0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2017-04-28 18:05 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On 04/28/17 18:48, Jeff Law wrote:
> On 04/23/2017 05:54 AM, Bernd Edlinger wrote:
>> Hi Jeff,
>>
>>
>> this patch tries to fix the handling of pic_offset_rtx +
>> const(unspec(symbol_ref)+ const_int) in may_trap_p,
>> and restores the original state of affairs in update_equiv_regs.
>>
>>
>> What do you think is it OK to extract the symbol_ref out
>> of the unspec in this way, or is does it need a target hook?
>>
>>
>> Patch works at least for x86_64 and arm.
>> Is it OK for trunk?
>>
>>
>> Bernd.
>>
>>
>> patch-pr79286.diff
>>
>>
>> 2017-04-23 Bernd Edlinger<bernd.edlinger@hotmail.de>
>>
>> rtl-optimizatoin/79286
>> * ira.c (update_equiv_regs): Revert to using may_tap_p again.
>> * rtlanal.c (rtx_addr_can_trap_p_1): SYMBOL_REF_FUNCTION_P
>> can never
>> trap. Extract constant offset from pic_offset_table_rtx +
>> const(unspec(symbol_ref)+int_val) and pic_offset_table_rtx +
>> const(unspec(symbol_ref)), otherwise RTL may trap.
> ISTM that rtx_addr_can_trap_p_1 is fundamentally broken in that
> references via pic_offset_table_rtx can certainly trap.
>
> Whether or not a given reference traps is a function of the size of
> table (not known until link time) and the CONST_INT within the address
> expression. So I'd be more inclined to remove the special casing of PIC
> addresses where entirely -- that seemed pretty risky during stage3, but
> now would be an appropriate time to tackle that.
>
> If we were to try and keep the special handling, you certainly can't
> depend on the form looking like (const(unspec(symbol_ref) + const_int).
>
> You could have high/lo_sums were and probably other forms too. We allow
> the backends to largely define what PIC address looks like.
>
Yes I agree, that is probably not worth it. So I could try to remove
the special handling of PIC+const and see what happens.
However the SYMBOL_REF_FUNCTION_P is another story, that part I would
like to keep: It happens quite often, already w/o -fpic that call
statements are using SYMBOL_REFs to ordinary (not weak) function
symbols, and may_trap returns 1 for these call statements wihch is IMHO
wrong.
Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-28 18:05 ` Bernd Edlinger
@ 2017-04-28 19:01 ` Jeff Law
2017-04-28 20:23 ` Bernd Edlinger
2017-04-29 9:27 ` Bernd Edlinger
0 siblings, 2 replies; 11+ messages in thread
From: Jeff Law @ 2017-04-28 19:01 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches
On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>
>
> Yes I agree, that is probably not worth it. So I could try to remove
> the special handling of PIC+const and see what happens.
>
> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
> like to keep: It happens quite often, already w/o -fpic that call
> statements are using SYMBOL_REFs to ordinary (not weak) function
> symbols, and may_trap returns 1 for these call statements wihch is IMHO
> wrong.
Hmm, thinking more about this, wasn't the original case a PIC referrence
for something like &x[BIGNUM].
Perhaps we could consider a PIC reference without other arithmetic as
safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you want
as well good deal many more PIC references as non-trapping.
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-28 19:01 ` Jeff Law
@ 2017-04-28 20:23 ` Bernd Edlinger
2017-04-28 20:27 ` Bernd Edlinger
2017-04-29 9:27 ` Bernd Edlinger
1 sibling, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2017-04-28 20:23 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On 04/28/17 20:46, Jeff Law wrote:
> On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>>
>>
>> Yes I agree, that is probably not worth it. So I could try to remove
>> the special handling of PIC+const and see what happens.
>>
>> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
>> like to keep: It happens quite often, already w/o -fpic that call
>> statements are using SYMBOL_REFs to ordinary (not weak) function
>> symbols, and may_trap returns 1 for these call statements wihch is IMHO
>> wrong.
> Hmm, thinking more about this, wasn't the original case a PIC referrence
> for something like &x[BIGNUM].
>
> Perhaps we could consider a PIC reference without other arithmetic as
> safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you want
> as well good deal many more PIC references as non-trapping.
>
> Jeff
Yes, IIRC it was a UNSPEC_GOTOFF.
I think it comes from legitimize_pic_address:
if (GET_CODE (addr) == PLUS)
{
new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, XEXP (addr, 0)),
UNSPEC_GOTOFF);
new_rtx = gen_rtx_PLUS (Pmode, new_rtx, XEXP (addr, 1));
}
else
new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, addr),
UNSPEC_GOTOFF);
new_rtx = gen_rtx_CONST (Pmode, new_rtx);
and it is somehow special, because it is a static value
that is accessed relative to the PIC register,
we know the bounds of the static object, the form of the
RTL may vary dependent on the target, of course, if the
form is not recognized, may_trap_p would behave as if
the PIC+const case was not there. Maybe I could check
that the SYMBOL_REF is a local value?
Everything else is accessing the address of an external
variable, this is arranged by the linker and should be safe.
Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-28 20:23 ` Bernd Edlinger
@ 2017-04-28 20:27 ` Bernd Edlinger
0 siblings, 0 replies; 11+ messages in thread
From: Bernd Edlinger @ 2017-04-28 20:27 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On 04/28/17 21:14, Bernd Edlinger wrote:
> On 04/28/17 20:46, Jeff Law wrote:
>> On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>>>
>>>
>>> Yes I agree, that is probably not worth it. So I could try to remove
>>> the special handling of PIC+const and see what happens.
>>>
>>> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
>>> like to keep: It happens quite often, already w/o -fpic that call
>>> statements are using SYMBOL_REFs to ordinary (not weak) function
>>> symbols, and may_trap returns 1 for these call statements wihch is IMHO
>>> wrong.
>> Hmm, thinking more about this, wasn't the original case a PIC referrence
>> for something like &x[BIGNUM].
>>
>> Perhaps we could consider a PIC reference without other arithmetic as
>> safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you want
>> as well good deal many more PIC references as non-trapping.
>>
>> Jeff
>
> Yes, IIRC it was a UNSPEC_GOTOFF.
> I think it comes from legitimize_pic_address:
>
> if (GET_CODE (addr) == PLUS)
> {
> new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, XEXP (addr, 0)),
> UNSPEC_GOTOFF);
> new_rtx = gen_rtx_PLUS (Pmode, new_rtx, XEXP (addr, 1));
> }
> else
> new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, addr),
> UNSPEC_GOTOFF);
>
> new_rtx = gen_rtx_CONST (Pmode, new_rtx);
>
> and it is somehow special, because it is a static value
> that is accessed relative to the PIC register,
> we know the bounds of the static object, the form of the
> RTL may vary dependent on the target, of course, if the
> form is not recognized, may_trap_p would behave as if
> the PIC+const case was not there. Maybe I could check
> that the SYMBOL_REF is a local value?
>
> Everything else is accessing the address of an external
> variable, this is arranged by the linker and should be safe.
>
>
Reading a bit further in legitimize_pic_address I see this:
new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, addr),
UNSPEC_GOT);
new_rtx = gen_rtx_CONST (Pmode, new_rtx);
if (TARGET_64BIT)
new_rtx = force_reg (Pmode, new_rtx);
new_rtx = gen_rtx_PLUS (Pmode, pic_offset_table_rtx, new_rtx);
new_rtx = gen_const_mem (Pmode, new_rtx);
set_mem_alias_set (new_rtx, ix86_GOT_alias_set ());
and gen_const_mem sets MEM_NOTRAP_P
furthermore in may_trap_p_1 we have:
case MEM:
/* Recognize specific pattern of stack checking probes. */
if (flag_stack_check
&& MEM_VOLATILE_P (x)
&& XEXP (x, 0) == stack_pointer_rtx)
return 1;
if (/* MEM_NOTRAP_P only relates to the actual position of the memory
reference; moving it out of context such as when moving code
when optimizing, might cause its address to become
invalid. */
code_changed
|| !MEM_NOTRAP_P (x))
{
HOST_WIDE_INT size = MEM_SIZE_KNOWN_P (x) ? MEM_SIZE (x) : 0;
return rtx_addr_can_trap_p_1 (XEXP (x, 0), 0, size,
GET_MODE (x), code_changed);
}
return 0;
So it is quite possible that the real pic refernces will not
go into rtx_addr_can_trap_p_1 at all.
Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-28 19:01 ` Jeff Law
2017-04-28 20:23 ` Bernd Edlinger
@ 2017-04-29 9:27 ` Bernd Edlinger
2017-05-12 16:49 ` [PING][PATCH][ " Bernd Edlinger
2017-06-23 4:35 ` [PATCH][ " Jeff Law
1 sibling, 2 replies; 11+ messages in thread
From: Bernd Edlinger @ 2017-04-29 9:27 UTC (permalink / raw)
To: Jeff Law, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]
On 04/28/17 20:46, Jeff Law wrote:
> On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>>
>>
>> Yes I agree, that is probably not worth it. So I could try to remove
>> the special handling of PIC+const and see what happens.
>>
>> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
>> like to keep: It happens quite often, already w/o -fpic that call
>> statements are using SYMBOL_REFs to ordinary (not weak) function
>> symbols, and may_trap returns 1 for these call statements wihch is IMHO
>> wrong.
> Hmm, thinking more about this, wasn't the original case a PIC referrence
> for something like &x[BIGNUM].
>
> Perhaps we could consider a PIC reference without other arithmetic as
> safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you want
> as well good deal many more PIC references as non-trapping.
>
Yes, I like this idea.
I tried to compile openssl with -m32 -fpic as an example, and counted
how often the mem[pic+const] is hit: that was 2353 times, all kind of
object refs.
Then I tried your idea, and only 54 unhandled pic refs remained, all of
them looking like this:
(plus:SI (reg:SI 107)
(const:SI (plus:SI (unspec:SI [
(symbol_ref:SI ("bf_init") [flags 0x2] <var_decl
0x2ac00f7bac60 bf_init>)
] UNSPEC_GOTOFF)
(const_int 4164 [0x1044]))))
I believe that is a negligible fall out from such a big code base.
Although the pic references do no longer reach the
SYMBOL_REF_FUNCTION_P in this version of the patch, I still see
that happening without -fpic option, so I left it as is.
Attached is the new version of my patch.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-pr79286.diff --]
[-- Type: text/x-patch; name="patch-pr79286.diff", Size: 1895 bytes --]
2017-04-29 Bernd Edlinger <bernd.edlinger@hotmail.de>
rtl-optimizatoin/79286
* ira.c (update_equiv_regs): Revert to using may_tap_p again.
* rtlanal.c (rtx_addr_can_trap_p_1): SYMBOL_REF_FUNCTION_P can never
trap. PIC register plus a const unspec without offset can never trap.
Index: gcc/ira.c
===================================================================
--- gcc/ira.c (revision 247397)
+++ gcc/ira.c (working copy)
@@ -3551,7 +3551,8 @@ update_equiv_regs (void)
if (DF_REG_DEF_COUNT (regno) == 1
&& note
&& !rtx_varies_p (XEXP (note, 0), 0)
- && def_dominates_uses (regno))
+ && (!may_trap_or_fault_p (XEXP (note, 0))
+ || def_dominates_uses (regno)))
{
rtx note_value = XEXP (note, 0);
remove_note (insn, note);
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c (revision 247397)
+++ gcc/rtlanal.c (working copy)
@@ -485,7 +485,7 @@ rtx_addr_can_trap_p_1 (const_rtx x, HOST_WIDE_INT
case SYMBOL_REF:
if (SYMBOL_REF_WEAK (x))
return 1;
- if (!CONSTANT_POOL_ADDRESS_P (x))
+ if (!CONSTANT_POOL_ADDRESS_P (x) && !SYMBOL_REF_FUNCTION_P (x))
{
tree decl;
HOST_WIDE_INT decl_size;
@@ -644,8 +644,11 @@ rtx_addr_can_trap_p_1 (const_rtx x, HOST_WIDE_INT
case PLUS:
/* An address is assumed not to trap if:
- - it is the pic register plus a constant. */
- if (XEXP (x, 0) == pic_offset_table_rtx && CONSTANT_P (XEXP (x, 1)))
+ - it is the pic register plus a const unspec without offset. */
+ if (XEXP (x, 0) == pic_offset_table_rtx
+ && GET_CODE (XEXP (x, 1)) == CONST
+ && GET_CODE (XEXP (XEXP (x, 1), 0)) == UNSPEC
+ && offset == 0)
return 0;
/* - or it is an address that can't trap plus a constant integer. */
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING][PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-29 9:27 ` Bernd Edlinger
@ 2017-05-12 16:49 ` Bernd Edlinger
2017-06-01 16:00 ` [PING**2][PATCH][ " Bernd Edlinger
[not found] ` <59f99a5b-e5db-7078-5f55-c4b42f9c4a8b@hotmail.de>
2017-06-23 4:35 ` [PATCH][ " Jeff Law
1 sibling, 2 replies; 11+ messages in thread
From: Bernd Edlinger @ 2017-05-12 16:49 UTC (permalink / raw)
To: Jeff Law, gcc-patches
Ping...
On 04/29/17 09:06, Bernd Edlinger wrote:
> On 04/28/17 20:46, Jeff Law wrote:
>> On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>>>
>>>
>>> Yes I agree, that is probably not worth it. So I could try to remove
>>> the special handling of PIC+const and see what happens.
>>>
>>> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
>>> like to keep: It happens quite often, already w/o -fpic that call
>>> statements are using SYMBOL_REFs to ordinary (not weak) function
>>> symbols, and may_trap returns 1 for these call statements wihch is IMHO
>>> wrong.
>> Hmm, thinking more about this, wasn't the original case a PIC referrence
>> for something like &x[BIGNUM].
>>
>> Perhaps we could consider a PIC reference without other arithmetic as
>> safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you want
>> as well good deal many more PIC references as non-trapping.
>>
>
> Yes, I like this idea.
>
> I tried to compile openssl with -m32 -fpic as an example, and counted
> how often the mem[pic+const] is hit: that was 2353 times, all kind of
> object refs.
>
> Then I tried your idea, and only 54 unhandled pic refs remained, all of
> them looking like this:
>
> (plus:SI (reg:SI 107)
> (const:SI (plus:SI (unspec:SI [
> (symbol_ref:SI ("bf_init") [flags 0x2] <var_decl
> 0x2ac00f7bac60 bf_init>)
> ] UNSPEC_GOTOFF)
> (const_int 4164 [0x1044]))))
>
> I believe that is a negligible fall out from such a big code base.
>
> Although the pic references do no longer reach the
> SYMBOL_REF_FUNCTION_P in this version of the patch, I still see
> that happening without -fpic option, so I left it as is.
>
>
> Attached is the new version of my patch.
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING**2][PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-05-12 16:49 ` [PING][PATCH][ " Bernd Edlinger
@ 2017-06-01 16:00 ` Bernd Edlinger
[not found] ` <59f99a5b-e5db-7078-5f55-c4b42f9c4a8b@hotmail.de>
1 sibling, 0 replies; 11+ messages in thread
From: Bernd Edlinger @ 2017-06-01 16:00 UTC (permalink / raw)
To: Jeff Law, gcc-patches
Ping...
On 05/12/17 18:48, Bernd Edlinger wrote:
> Ping...
>
> On 04/29/17 09:06, Bernd Edlinger wrote:
>> On 04/28/17 20:46, Jeff Law wrote:
>>> On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>>>>
>>>>
>>>> Yes I agree, that is probably not worth it. So I could try to remove
>>>> the special handling of PIC+const and see what happens.
>>>>
>>>> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
>>>> like to keep: It happens quite often, already w/o -fpic that call
>>>> statements are using SYMBOL_REFs to ordinary (not weak) function
>>>> symbols, and may_trap returns 1 for these call statements wihch is IMHO
>>>> wrong.
>>> Hmm, thinking more about this, wasn't the original case a PIC referrence
>>> for something like &x[BIGNUM].
>>>
>>> Perhaps we could consider a PIC reference without other arithmetic as
>>> safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you want
>>> as well good deal many more PIC references as non-trapping.
>>>
>>
>> Yes, I like this idea.
>>
>> I tried to compile openssl with -m32 -fpic as an example, and counted
>> how often the mem[pic+const] is hit: that was 2353 times, all kind of
>> object refs.
>>
>> Then I tried your idea, and only 54 unhandled pic refs remained, all of
>> them looking like this:
>>
>> (plus:SI (reg:SI 107)
>> (const:SI (plus:SI (unspec:SI [
>> (symbol_ref:SI ("bf_init") [flags 0x2] <var_decl
>> 0x2ac00f7bac60 bf_init>)
>> ] UNSPEC_GOTOFF)
>> (const_int 4164 [0x1044]))))
>>
>> I believe that is a negligible fall out from such a big code base.
>>
>> Although the pic references do no longer reach the
>> SYMBOL_REF_FUNCTION_P in this version of the patch, I still see
>> that happening without -fpic option, so I left it as is.
>>
>>
>> Attached is the new version of my patch.
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING**3][PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
[not found] ` <59f99a5b-e5db-7078-5f55-c4b42f9c4a8b@hotmail.de>
@ 2017-06-14 12:43 ` Bernd Edlinger
0 siblings, 0 replies; 11+ messages in thread
From: Bernd Edlinger @ 2017-06-14 12:43 UTC (permalink / raw)
To: Jeff Law, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]
Ping...
I attached the patch again for your reference.
Is it OK for trunk?
Thanks
Bernd.
On 06/01/17 17:59, Bernd Edlinger wrote:
> Ping...
>
> On 05/12/17 18:48, Bernd Edlinger wrote:
>> Ping...
>>
>> On 04/29/17 09:06, Bernd Edlinger wrote:
>>> On 04/28/17 20:46, Jeff Law wrote:
>>>> On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>>>>>
>>>>>
>>>>> Yes I agree, that is probably not worth it. So I could try to remove
>>>>> the special handling of PIC+const and see what happens.
>>>>>
>>>>> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
>>>>> like to keep: It happens quite often, already w/o -fpic that call
>>>>> statements are using SYMBOL_REFs to ordinary (not weak) function
>>>>> symbols, and may_trap returns 1 for these call statements wihch is
>>>>> IMHO
>>>>> wrong.
>>>> Hmm, thinking more about this, wasn't the original case a PIC
>>>> referrence
>>>> for something like &x[BIGNUM].
>>>>
>>>> Perhaps we could consider a PIC reference without other arithmetic as
>>>> safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you
>>>> want
>>>> as well good deal many more PIC references as non-trapping.
>>>>
>>>
>>> Yes, I like this idea.
>>>
>>> I tried to compile openssl with -m32 -fpic as an example, and counted
>>> how often the mem[pic+const] is hit: that was 2353 times, all kind of
>>> object refs.
>>>
>>> Then I tried your idea, and only 54 unhandled pic refs remained, all of
>>> them looking like this:
>>>
>>> (plus:SI (reg:SI 107)
>>> (const:SI (plus:SI (unspec:SI [
>>> (symbol_ref:SI ("bf_init") [flags 0x2] <var_decl
>>> 0x2ac00f7bac60 bf_init>)
>>> ] UNSPEC_GOTOFF)
>>> (const_int 4164 [0x1044]))))
>>>
>>> I believe that is a negligible fall out from such a big code base.
>>>
>>> Although the pic references do no longer reach the
>>> SYMBOL_REF_FUNCTION_P in this version of the patch, I still see
>>> that happening without -fpic option, so I left it as is.
>>>
>>>
>>> Attached is the new version of my patch.
>>>
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-pr79286.diff --]
[-- Type: text/x-patch; name="patch-pr79286.diff", Size: 1905 bytes --]
2017-04-29 Bernd Edlinger <bernd.edlinger@hotmail.de>
rtl-optimizatoin/79286
* ira.c (update_equiv_regs): Revert to using may_trap_or_fault_p again.
* rtlanal.c (rtx_addr_can_trap_p_1): SYMBOL_REF_FUNCTION_P can never
trap. PIC register plus a const unspec without offset can never trap.
Index: gcc/ira.c
===================================================================
--- gcc/ira.c (revision 247397)
+++ gcc/ira.c (working copy)
@@ -3551,7 +3551,8 @@ update_equiv_regs (void)
if (DF_REG_DEF_COUNT (regno) == 1
&& note
&& !rtx_varies_p (XEXP (note, 0), 0)
- && def_dominates_uses (regno))
+ && (!may_trap_or_fault_p (XEXP (note, 0))
+ || def_dominates_uses (regno)))
{
rtx note_value = XEXP (note, 0);
remove_note (insn, note);
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c (revision 247397)
+++ gcc/rtlanal.c (working copy)
@@ -485,7 +485,7 @@ rtx_addr_can_trap_p_1 (const_rtx x, HOST_WIDE_INT
case SYMBOL_REF:
if (SYMBOL_REF_WEAK (x))
return 1;
- if (!CONSTANT_POOL_ADDRESS_P (x))
+ if (!CONSTANT_POOL_ADDRESS_P (x) && !SYMBOL_REF_FUNCTION_P (x))
{
tree decl;
HOST_WIDE_INT decl_size;
@@ -644,8 +644,11 @@ rtx_addr_can_trap_p_1 (const_rtx x, HOST_WIDE_INT
case PLUS:
/* An address is assumed not to trap if:
- - it is the pic register plus a constant. */
- if (XEXP (x, 0) == pic_offset_table_rtx && CONSTANT_P (XEXP (x, 1)))
+ - it is the pic register plus a const unspec without offset. */
+ if (XEXP (x, 0) == pic_offset_table_rtx
+ && GET_CODE (XEXP (x, 1)) == CONST
+ && GET_CODE (XEXP (XEXP (x, 1), 0)) == UNSPEC
+ && offset == 0)
return 0;
/* - or it is an address that can't trap plus a constant integer. */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs
2017-04-29 9:27 ` Bernd Edlinger
2017-05-12 16:49 ` [PING][PATCH][ " Bernd Edlinger
@ 2017-06-23 4:35 ` Jeff Law
1 sibling, 0 replies; 11+ messages in thread
From: Jeff Law @ 2017-06-23 4:35 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches
On 04/29/2017 01:06 AM, Bernd Edlinger wrote:
> On 04/28/17 20:46, Jeff Law wrote:
>> On 04/28/2017 11:27 AM, Bernd Edlinger wrote:
>>> Yes I agree, that is probably not worth it. So I could try to remove
>>> the special handling of PIC+const and see what happens.
>>>
>>> However the SYMBOL_REF_FUNCTION_P is another story, that part I would
>>> like to keep: It happens quite often, already w/o -fpic that call
>>> statements are using SYMBOL_REFs to ordinary (not weak) function
>>> symbols, and may_trap returns 1 for these call statements wihch is IMHO
>>> wrong.
>> Hmm, thinking more about this, wasn't the original case a PIC referrence
>> for something like &x[BIGNUM].
>>
>> Perhaps we could consider a PIC reference without other arithmetic as
>> safe. That would likely pick up the SYMBOL_REF_FUNCTION_P case you want
>> as well good deal many more PIC references as non-trapping.
>>
> Yes, I like this idea.
>
> I tried to compile openssl with -m32 -fpic as an example, and counted
> how often the mem[pic+const] is hit: that was 2353 times, all kind of
> object refs.
>
> Then I tried your idea, and only 54 unhandled pic refs remained, all of
> them looking like this:
>
> (plus:SI (reg:SI 107)
> (const:SI (plus:SI (unspec:SI [
> (symbol_ref:SI ("bf_init") [flags 0x2] <var_decl
> 0x2ac00f7bac60 bf_init>)
> ] UNSPEC_GOTOFF)
> (const_int 4164 [0x1044]))))
>
> I believe that is a negligible fall out from such a big code base.
>
> Although the pic references do no longer reach the
> SYMBOL_REF_FUNCTION_P in this version of the patch, I still see
> that happening without -fpic option, so I left it as is.
>
>
> Attached is the new version of my patch.
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
>
>
> patch-pr79286.diff
>
>
> 2017-04-29 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> rtl-optimizatoin/79286
> * ira.c (update_equiv_regs): Revert to using may_tap_p again.
> * rtlanal.c (rtx_addr_can_trap_p_1): SYMBOL_REF_FUNCTION_P can never
> trap. PIC register plus a const unspec without offset can never trap.
OK. Sorry for the delay. I've been swamped.
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-06-23 4:35 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-23 18:33 [PATCH][ PR rtl-optimization/79286] Drop may_trap_p exception to testing dominance in update_equiv_regs Bernd Edlinger
2017-04-28 17:17 ` Jeff Law
2017-04-28 18:05 ` Bernd Edlinger
2017-04-28 19:01 ` Jeff Law
2017-04-28 20:23 ` Bernd Edlinger
2017-04-28 20:27 ` Bernd Edlinger
2017-04-29 9:27 ` Bernd Edlinger
2017-05-12 16:49 ` [PING][PATCH][ " Bernd Edlinger
2017-06-01 16:00 ` [PING**2][PATCH][ " Bernd Edlinger
[not found] ` <59f99a5b-e5db-7078-5f55-c4b42f9c4a8b@hotmail.de>
2017-06-14 12:43 ` [PING**3][PATCH][ " Bernd Edlinger
2017-06-23 4:35 ` [PATCH][ " Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).