[Bug target/40956] New: GCSE opportunity in if statement

[Bug target/40956] New: GCSE opportunity in if statement

[carrot at google dot com]

Compile the following function with options -Os -mthumb -march=armv5te
-frename-registers

int foo(int p, int* q)
{
  if (p!=9)
    *q = 0;
  else
    *(q+1) = 0;
  return 3;
}

GCC generates:

        push    {lr}
        cmp     r0, #9         // D
        beq     .L2
        mov     r3, #0         // A
        str     r3, [r1]
        b       .L3
.L2:
        mov     r0, #0         // B
        str     r0, [r1, #4]   // C
.L3:
        mov     r0, #3
        pop     {pc}

If we replace r0 with r3 in instructions B and C, then A and B will be same. So
we can move the same instruction before the instruction D and reduce 1
instruction.

Is it a gcse opportunity?


-- 
           Summary: GCSE opportunity in if statement
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: carrot at google dot com
 GCC build triplet: i686-linux
  GCC host triplet: i686-linux
GCC target triplet: arm-eabi


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

[Bug target/40956] GCSE opportunity in if statement

[carrot at google dot com]

------- Comment #1 from carrot at google dot com  2009-08-03 22:55 -------
Created an attachment (id=18294)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=18294&action=view)
test case


-- 


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

[Bug target/40956] GCSE opportunity in if statement

[steven at gcc dot gnu dot org]

------- Comment #2 from steven at gcc dot gnu dot org  2009-08-04 09:05 -------
Hoisting

*** This bug has been marked as a duplicate of 23286 ***


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |DUPLICATE


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

[Bug target/40956] GCSE opportunity in if statement

[davidxl at gcc dot gnu dot org]

------- Comment #3 from davidxl at gcc dot gnu dot org  2009-12-23 19:37 -------
This bug is ARM specific (thumb) mode. In x86, the hoisting is unnecessary as
the move instruction support the imm form. 

The issue here is more in the GIMPLE canonicalization (target specific). In
this case, the IR should be in the following form to expose the hoisting.

if (...) {
   temp = 0;
   *p = temp;
}
else
{
   temp = 0;
   *(p+1) = temp;
}


-- 

davidxl at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|DUPLICATE                   |


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

[Bug rtl-optimization/40956] Constants are never candidates for hoisting

[steven at gcc dot gnu dot org]

------- Comment #4 from steven at gcc dot gnu dot org  2010-03-19 18:39 -------
Comment #3 makes no sense: There is no such thing as target specific GIMPLE
canonicalization. And also there is no hoisting for GIMPLE so the form of the
IR given in comment #3 wouldn't make a difference.

Even without -frename-registers, the extra insn is there. GCC trunk revision
157579 generates the following code with "-Os -mthumb -march=armv5te":

foo:
        push    {lr}
        cmp     r0, #9
        beq     .L2
        mov     r3, #0
        str     r3, [r1]
        b       .L3
.L2:
        mov     r3, #0
        str     r3, [r1, #4]
.L3:
        mov     r0, #3
        @ sp needed for prologue
        pop     {pc}

Here the "mov r3, #0" after ".L2:" and after "beq .L2" could be unified e.g.
with hoisting or with head-merging (inverse of tail merging).


Just before the RTL hoisting pass, the code looks like this (.152r.cprop1
dump):

    5 NOTE_INSN_BASIC_BLOCK
    2 r135:SI=r0:SI
      REG_DEAD: r0:SI
    3 r136:SI=r1:SI
      REG_DEAD: r1:SI
    4 NOTE_INSN_FUNCTION_BEG
    7 pc={(r135:SI==0x9)?L13:pc}
      REG_DEAD: r135:SI
      REG_BR_PROB: 0xec6
    8 NOTE_INSN_BASIC_BLOCK
    9 r137:SI=0x0
   10 [r136:SI]=r137:SI
      REG_EQUAL: 0x0
      REG_DEAD: r137:SI
      REG_DEAD: r136:SI
L13:
   14 NOTE_INSN_BASIC_BLOCK
   15 r138:SI=0x0
   16 [r136:SI+0x4]=r138:SI
      REG_EQUAL: 0x0
      REG_DEAD: r138:SI
      REG_DEAD: r136:SI
L17:
   18 NOTE_INSN_BASIC_BLOCK
   23 r0:SI=0x3
   26 use r0:SI

Note the insns "9 r137:SI=0x0" and "15 r138:SI=0x0". This could be hoisted in
the RTL HOIST pass. But this doesn't happen because the hoisting pass doesn't
record constants as hoisting candidates. The "expression passes" in gcse.c (PRE
and HOIST) ignore all instructions of the form "(set (reg) (const))" because
want_to_gcse_p() returns false for them. This is a deliberate decision in the
implementation.

Thus, there is nothing target specific about this problem. The analysis of
comment #3 makes no sense, but this is also not really a dup of bug 23286.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |33828
              nThis|                            |
           Severity|normal                      |enhancement
             Status|UNCONFIRMED                 |NEW
          Component|target                      |rtl-optimization
     Ever Confirmed|0                           |1
  GCC build triplet|i686-linux                  |
   GCC host triplet|i686-linux                  |
   Last reconfirmed|0000-00-00 00:00:00         |2010-03-19 18:39:40
               date|                            |
            Summary|GCSE opportunity in if      |Constants are never
                   |statement                   |candidates for hoisting


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

[Bug rtl-optimization/40956] Constants are never candidates for hoisting

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #5 from mkuvyrkov at gcc dot gnu dot org  2010-06-19 19:57 -------
I'm working on this bug among other improvements to RTL hoist pass.

My plan is to enable hoisting of such simple constants, but only on very short
distances, like 3-5 instructions, tunable through a new parameter.  Targets
like x86 can disable simple constant hoisting altogether, while targets like
ARM would be able to enable it when optimizing for size.


-- 

mkuvyrkov at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |mkuvyrkov at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2010-03-19 18:39:40         |2010-06-19 19:57:16
               date|                            |


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

[Bug rtl-optimization/40956] Constants are never candidates for hoisting

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #6 from mkuvyrkov at gcc dot gnu dot org  2010-07-27 19:35 -------
Subject: Bug 40956

Author: mkuvyrkov
Date: Tue Jul 27 19:34:55 2010
New Revision: 162590

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162590
Log:
        PR rtl-optimization/40956
        PR target/42495
        PR middle-end/42574
        * gcse.c (compute_code_hoist_vbeinout): Consider more expressions
        for hoisting.
        (hoist_code): Count occurences in current block too.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gcse.c


-- 


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

[Bug rtl-optimization/40956] Constants are never candidates for hoisting

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #7 from mkuvyrkov at gcc dot gnu dot org  2010-07-27 19:46 -------
Subject: Bug 40956

Author: mkuvyrkov
Date: Tue Jul 27 19:46:26 2010
New Revision: 162596

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162596
Log:
        PR rtl-optimization/40956
        * config/arm/arm.c (thumb1_size_rtx_costs): Fix cost of simple
        constants.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c


-- 


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

[Bug rtl-optimization/40956] Constants are never candidates for hoisting

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #8 from mkuvyrkov at gcc dot gnu dot org  2010-07-27 21:06 -------
Subject: Bug 40956

Author: mkuvyrkov
Date: Tue Jul 27 21:06:31 2010
New Revision: 162600

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162600
Log:
        PR rtl-optimization/40956
        PR target/42495
        PR middle-end/42574
        * gcc.target/arm/pr40956.c, gcc.target/arm/pr42495.c,
        * gcc.target/arm/pr42574.c: Add tests.

Added:
    trunk/gcc/testsuite/gcc.target/arm/pr40956.c
    trunk/gcc/testsuite/gcc.target/arm/pr42495.c
    trunk/gcc/testsuite/gcc.target/arm/pr42574.c
Modified:
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug rtl-optimization/40956] Constants are never candidates for hoisting

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #9 from mkuvyrkov at gcc dot gnu dot org  2010-07-27 21:10 -------
Should be fixed now by the above patch series.


-- 

mkuvyrkov at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


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