[Bug middle-end/65391] New: unnecessary load of conditionally updated pointer in loop

[Bug middle-end/65391] New: unnecessary load of conditionally updated pointer in loop

[acsawdey at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65391

            Bug ID: 65391
           Summary: unnecessary load of conditionally updated pointer in
                    loop
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acsawdey at linux dot vnet.ibm.com
                CC: dje at gcc dot gnu.org, pthaugen at us dot ibm.com

When compiling for powerpc64 or powerpc64le with -O3, a load and store of
*o_ptr is done inside the loop. 

If you remove the if statement and make the update unconditional, then the load
goes away and the store is deferred until after the loop.

If you remove the __restrict__ keywords, then the store remains in the loop in
either case as expected. However the load is still done in the loop if the
update is conditional.

void compute_object_gain(long * __restrict__ p_ptr, long * __restrict__ o_ptr,
long g_order)
{
    long a_binding;
    *o_ptr = 0;
    while(*p_ptr!=0) {
       a_binding = *p_ptr;
       if(a_binding <= g_order)
          *o_ptr += a_binding;
        p_ptr++;
    }
}

This behavior is consistent on 4.1, 4.5, 4.6, 4.7, 4.8, and 5.0 (trunk 220806).

[Bug middle-end/65391] unnecessary load of conditionally updated pointer in loop

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65391

David Edelsohn <dje at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-03-11
     Ever confirmed|0                           |1
      Known to fail|                            |4.1.0, 4.5.0, 4.6.0, 4.7.0,
                   |                            |4.8.0

--- Comment #1 from David Edelsohn <dje at gcc dot gnu.org> ---
Confirmed.

[Bug middle-end/65391] unnecessary load of conditionally updated pointer in loop

[acsawdey at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65391

--- Comment #2 from Aaron Sawdey <acsawdey at linux dot vnet.ibm.com> ---
Asm for the test case as in the description (load/store of *o_ptr for every
update):

compute_object_gain:
        ld 9,0(3)
        li 10,0
        std 10,0(4)
        cmpdi 7,9,0
        beqlr 7
        .p2align 5,,31
.L6:
        cmpd 7,5,9
        blt 7,.L3
        ld 10,0(4)
        add 9,10,9
        std 9,0(4)
.L3:
        ldu 9,8(3)
        cmpdi 7,9,0
        bne 7,.L6
        blr

Same test case, but with __restrict__ removed (essentially no difference):
compute_object_gain:
        li 9,0
        std 9,0(4)
        ld 9,0(3)
        cmpdi 7,9,0
        beqlr 7
        .p2align 5,,31
.L6:
        cmpd 7,5,9
        blt 7,.L3
        ld 10,0(4)
        add 9,10,9
        std 9,0(4)
.L3:
        ldu 9,8(3)
        cmpdi 7,9,0
        bne 7,.L6
        blr

Remove the if() and add __restrict__ back (no load or store of *o_ptr in the
loop):
compute_object_gain:
        ld 9,0(3)
        li 8,0
        li 10,0
        std 8,0(4)
        cmpdi 7,9,0
        beqlr 7
        .p2align 4,,15
.L5:
        add 10,10,9
        ldu 9,8(3)
        cmpdi 7,9,0
        bne 7,.L5
        std 10,0(4)
        blr

Remove both the if() and __restrict__ (now store to *o_ptr is in the loop but
no load):
compute_object_gain:
        li 9,0
        li 10,0
        std 9,0(4)
        ld 9,0(3)
        cmpdi 7,9,0
        beqlr 7
        .p2align 5,,31
.L5:
        add 10,10,9
        std 10,0(4)
        ldu 9,8(3)
        cmpdi 7,9,0
        bne 7,.L5
        blr

[Bug middle-end/65391] unnecessary load of conditionally updated pointer in loop

[acsawdey at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65391

--- Comment #3 from Aaron Sawdey <acsawdey at linux dot vnet.ibm.com> ---
I tried applying the patch that Thomas posted for 64616
(https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00272.html) to trunk 221350 but
that did not change the generated code for this test case on powerpc64le.

[Bug tree-optimization/65391] missed store motion for conditionally updated pointer in loop

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65391

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
                 CC|                            |rguenth at gcc dot gnu.org
          Component|middle-end                  |tree-optimization
            Summary|unnecessary load of         |missed store motion for
                   |conditionally updated       |conditionally updated
                   |pointer in loop             |pointer in loop

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
The issue is that invariant/store motion doesn't see the unconditional store
to *o_ptr before the loop and thus thinks that the conditional one may trap.

LIM doesn't consider stores/loads in blocks that are post-dominated by
the loop header to alter that loops "effectively accessed" set of references.
I think considering only the loop preheader is good enough - but we also do not
analyze memory references in non-loop areas (loop preheaders would need to be
added here as well).