diff mbox

Improve RTL ifcvt for empty else_bb (PR rtl-optimization/80491)

Message ID 20170426112537.GO1809@tucnak
State New
Headers show

Commit Message

Jakub Jelinek April 26, 2017, 11:25 a.m. UTC
On Tue, Apr 25, 2017 at 09:31:00PM +0200, Jakub Jelinek wrote:
> This patch let us search for x's setter earlier in the bb.
> During testing I found that modified_in_p/modified_in_between_p don't
> actually take into account calls that could change MEMs, so the patch
> handles that too.

Or shall we just:

instead of the call_crossed hacks?  Then modified_between_p and
modified_in_p would return true for !MEM_READONLY_P MEMs crossing a call.

	Jakub

Comments

Jeff Law April 28, 2017, 6:32 p.m. UTC | #1
On 04/26/2017 05:25 AM, Jakub Jelinek wrote:
> On Tue, Apr 25, 2017 at 09:31:00PM +0200, Jakub Jelinek wrote:
>> This patch let us search for x's setter earlier in the bb.
>> During testing I found that modified_in_p/modified_in_between_p don't
>> actually take into account calls that could change MEMs, so the patch
>> handles that too.
> 
> Or shall we just:
> --- gcc/alias.c	2017-04-25 15:51:31.072923325 +0200
> +++ gcc/alias.c	2017-04-26 13:23:55.595048464 +0200
> @@ -3221,6 +3221,8 @@ memory_modified_in_insn_p (const_rtx mem
>   {
>     if (!INSN_P (insn))
>       return false;
> +  if (CALL_P (insn))
> +    return true;
>     memory_modified = false;
>     note_stores (PATTERN (insn), memory_modified_1, CONST_CAST_RTX(mem));
>     return memory_modified;
> 
> instead of the call_crossed hacks?  Then modified_between_p and
> modified_in_p would return true for !MEM_READONLY_P MEMs crossing a call.
This seems like a good idea as well.  I'm a bit surprised we're not 
handling it this way already.  If you want to go w ith this and simplify 
the patch for 80491, that's fine too.

jeff
Steven Bosscher April 29, 2017, 8:43 p.m. UTC | #2
On Wed, Apr 26, 2017 at 1:25 PM, Jakub Jelinek wrote:
> Or shall we just:
> --- gcc/alias.c 2017-04-25 15:51:31.072923325 +0200
> +++ gcc/alias.c 2017-04-26 13:23:55.595048464 +0200
> @@ -3221,6 +3221,8 @@ memory_modified_in_insn_p (const_rtx mem
>  {
>    if (!INSN_P (insn))
>      return false;
> +  if (CALL_P (insn))
> +    return true;

+"&& ! RTL_CONST_OR_PURE_CALL_P (insn)" ?

Ciao!
Steven




>    memory_modified = false;
>    note_stores (PATTERN (insn), memory_modified_1, CONST_CAST_RTX(mem));
>    return memory_modified;
>
> instead of the call_crossed hacks?  Then modified_between_p and
> modified_in_p would return true for !MEM_READONLY_P MEMs crossing a call.
>
>         Jakub
Jakub Jelinek April 29, 2017, 8:53 p.m. UTC | #3
On Sat, Apr 29, 2017 at 10:43:52PM +0200, Steven Bosscher wrote:
> On Wed, Apr 26, 2017 at 1:25 PM, Jakub Jelinek wrote:
> > Or shall we just:
> > --- gcc/alias.c 2017-04-25 15:51:31.072923325 +0200
> > +++ gcc/alias.c 2017-04-26 13:23:55.595048464 +0200
> > @@ -3221,6 +3221,8 @@ memory_modified_in_insn_p (const_rtx mem
> >  {
> >    if (!INSN_P (insn))
> >      return false;
> > +  if (CALL_P (insn))
> > +    return true;
> 
> +"&& ! RTL_CONST_OR_PURE_CALL_P (insn)" ?

Without detailed analysis of the MEM I'm not convinced it is safe.
I believe even const or pure calls invalidate e.g. the argument stack slots,
at least in the ABIs where stack slots are owned by callee, not caller.

	Jakub
diff mbox

Patch

--- gcc/alias.c	2017-04-25 15:51:31.072923325 +0200
+++ gcc/alias.c	2017-04-26 13:23:55.595048464 +0200
@@ -3221,6 +3221,8 @@  memory_modified_in_insn_p (const_rtx mem
 {
   if (!INSN_P (insn))
     return false;
+  if (CALL_P (insn))
+    return true;
   memory_modified = false;
   note_stores (PATTERN (insn), memory_modified_1, CONST_CAST_RTX(mem));
   return memory_modified;