[Bug c/27313] New: Does not emit conditional moves for stores

[Bug c/27313] New: Does not emit conditional moves for stores

[dwarak dot rajagopal at amd dot com]

int cmov(int* A ,int B ,int C ,int* D ,int* E ,int F ,int g) {
  int k,f;
  for (k = 1; k <= 1000; k++) {
    A[k] = B+C;
    g = D[k-1] + E[k-1];
    if (g > A[k])  A[k]=g;      /* This is not converted to cmov*/
    f += g;
  }
  return f;
}

In the above code, the if-then statement is not converted to conditional move.
It fails for "noce_mem_write_may_trap_or_fault_p ()" condition in "ifcvt.c" as
it thinks that there is a chance for A[k] access to trap.
The fact here is that in this case, A[k] will never trap because the A[k] is
already been written once along the path from Entry to the "A[k] = g". So it is
safe to convert it to a cmov statement. Though there might be two extra moves
(mem to reg and vice versa) statement, it is still better to avoid the branch
especially if it is unpredictable data like for the eg above.

There is a typical case like this in Spec 2006 456.hmmer benchmark. Using
contional moves will make the code faster by 13%-17%. 

-Dwarak


-- 
           Summary: Does not emit conditional moves for stores
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: dwarak dot rajagopal at amd dot com
 GCC build triplet: x86_64
  GCC host triplet: x86_64
GCC target triplet: x86_64


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313

[Bug middle-end/27313] Does not emit conditional moves for stores

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2006-04-25 17:38 -------
Confirmed, if cvt should have a way to track if a memory write has happened.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c                           |middle-end
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-04-25 17:38:13
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313

[Bug middle-end/27313] Does not emit conditional moves for stores

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2006-04-25 18:38 -------
The other way of getting this is to have the code converted so there is only
one store instead of two:

int cmov(int* A ,int B ,int C ,int* D ,int* E ,int F ,int g) {
  int k,f;
  for (k = 1; k <= 1000; k++) {
    int t = B+C;
    g = D[k-1] + E[k-1];
    if (g > t)  t=g;      /* This is not converted to cmov*/
    A[K] = t;
    f += g;
  }
  return f;
}
Which is most likely better anyways as one it is smaller.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313

[Bug middle-end/27313] Does not emit conditional moves for stores

[dwarak dot rajagopal at amd dot com]

------- Comment #3 from dwarak dot rajagopal at amd dot com  2006-04-25 19:07 -------
Yes this is true. The example I posted was a simplest case where it fails.
Below mmight be a typical case where you have to do two stores. 
int cmov(int* A ,int B ,int C ,int* D ,int* E ,int F ,int g) {
  int k,f;
  for (k = 1; k <= 1000; k++) {
    A[k] = B+C;
    D[k] = C; /* D[k] may alias with A[k] */ 
    g = D[k-1] + E[k-1];
    if (g > A[k])  A[k]=g;      /* This is not converted to cmov*/
    f += g;
  }
  return f;
}

In this case, you cannot reduce the number of stores (becasue D[k] may alias
with A[k]) but you still want the if conversion to take place. I think it is
good to have a mechanism to track if a memory is already been written in ifcvt.
I'm not sure how it can be done at this level though.  

-Dwarak


(In reply to comment #2)
> The other way of getting this is to have the code converted so there is only
> one store instead of two:
> 
> int cmov(int* A ,int B ,int C ,int* D ,int* E ,int F ,int g) {
>   int k,f;
>   for (k = 1; k <= 1000; k++) {
>     int t = B+C;
>     g = D[k-1] + E[k-1];
>     if (g > t)  t=g;      /* This is not converted to cmov*/
>     A[K] = t;
>     f += g;
>   }
>   return f;
> }
> Which is most likely better anyways as one it is smaller.
> 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313

[Bug middle-end/27313] Does not emit conditional moves for stores

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2006-09-21 13:07 -------
We have a flag (TREE_THIS_NOTRAP) to annotate ARRAY_REFs and INDIRECT_REFs.  It
could be used to guide tree-ifcvt to do the transformation.  Can we have
if-converted stores at the tree level at all?


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313

[Bug middle-end/27313] Does not emit conditional moves for stores

[steven at gcc dot gnu dot org]

------- Comment #5 from steven at gcc dot gnu dot org  2006-11-11 16:46 -------
You can't have if-converted stores at the tree level.

Forms of store motion, like the example of comment #2, ought to get implemented
at the tree level somewhen, though.  AFAIU this is currently very hard to do,
but the mem-ssa and new VN projects should help simplifying the job.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313

[Bug middle-end/27313] Does not emit conditional moves for stores

[wilson at gcc dot gnu dot org]

------- Comment #6 from wilson at gcc dot gnu dot org  2009-05-09 01:21 -------
It looks like this was fixed by Michael Matz here:
http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01978.html

This patch was added to gcc-4.3, and the gcc-4.2 branch is closed, so I think
this bug should be closed resolved.


-- 

wilson at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilson at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313

[Bug middle-end/27313] Does not emit conditional moves for stores

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2009-05-09 12:59 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27313