[rtl-optimization] : Fix PR54457, [x32] Fail to combine 64bit index + constant

Submitted by Uros Bizjak on Sept. 27, 2012, 1:04 p.m.

Details

Message ID CAFULd4aSLiau+Qa9vC_eUsPrRKwvAqYt=uXxGi5Sj5GphCYMmA@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Sept. 27, 2012, 1:04 p.m.
On Wed, Sep 26, 2012 at 11:22 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> I agree (subreg:M (op:N A C) 0) to (op:M (subreg:N (A 0)) C) is
>> a good transformation, but why do we need to handle as special
>> the case where the subreg is itself the operand of a plus or minus?
>> I think it should happen regardless of where the subreg occurs.
>
> Don't we need to restrict this to the low part though?

I have tried this approach with attached patch.  Unfortunately,
although it survived bootstrap without libjava on x86_64, it failed
building libjava with:

/home/uros/gcc-svn/trunk/libjava/classpath/javax/swing/plaf/basic/BasicSliderUI.java:1299:0:
error: insn does not satisfy its constraints:
   }
 ^
(insn 237 398 399 7 (set (reg:SI 1 dx [125])
        (plus:SI (subreg:SI (mult:DI (reg:DI 1 dx [orig:72 D.78627 ] [72])
                    (const_int 2 [0x2])) 0)
            (reg:SI 5 di)))
/home/uros/gcc-svn/trunk/libjava/classpath/javax/swing/plaf/basic/BasicSliderUI.java:1271
240 {*leasi}
     (expr_list:REG_DEAD (reg:DI 5 di)
        (nil)))

Original RTX was (subreg:SI (plus:DI (mult:DI (...) reg:DI))), which
is valid RTX pattern for lea insn, the above is not.

Due to these problems, I think the safer approach is to limit the
transformation to (plus:SI (subreg:SI (plus:DI (...) 0)) RTXes, as was
the case with original patch. This approach would fix a specific
problem where simplify_plus_minus is not able to simplify the combined
RTX at combine time. Please note, that combined RTXes are always
checked for correctness at combine pass.

Uros.

Comments

Richard Sandiford Sept. 27, 2012, 2:25 p.m.
Uros Bizjak <ubizjak@gmail.com> writes:
> On Wed, Sep 26, 2012 at 11:22 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> I agree (subreg:M (op:N A C) 0) to (op:M (subreg:N (A 0)) C) is
>>> a good transformation, but why do we need to handle as special
>>> the case where the subreg is itself the operand of a plus or minus?
>>> I think it should happen regardless of where the subreg occurs.
>>
>> Don't we need to restrict this to the low part though?
>
> I have tried this approach with attached patch.  Unfortunately,
> although it survived bootstrap without libjava on x86_64, it failed
> building libjava with:
>
> /home/uros/gcc-svn/trunk/libjava/classpath/javax/swing/plaf/basic/BasicSliderUI.java:1299:0:
> error: insn does not satisfy its constraints:
>    }
>  ^
> (insn 237 398 399 7 (set (reg:SI 1 dx [125])
>         (plus:SI (subreg:SI (mult:DI (reg:DI 1 dx [orig:72 D.78627 ] [72])
>                     (const_int 2 [0x2])) 0)
>             (reg:SI 5 di)))
> /home/uros/gcc-svn/trunk/libjava/classpath/javax/swing/plaf/basic/BasicSliderUI.java:1271
> 240 {*leasi}
>      (expr_list:REG_DEAD (reg:DI 5 di)
>         (nil)))
>
> Original RTX was (subreg:SI (plus:DI (mult:DI (...) reg:DI))), which
> is valid RTX pattern for lea insn, the above is not.
>
> Due to these problems, I think the safer approach is to limit the
> transformation to (plus:SI (subreg:SI (plus:DI (...) 0)) RTXes, as was
> the case with original patch. This approach would fix a specific
> problem where simplify_plus_minus is not able to simplify the combined
> RTX at combine time. Please note, that combined RTXes are always
> checked for correctness at combine pass.

I think instead the (subreg (plus ...)) handling should be applied
to (subreg (mult ...)) too.  IMO the correct form of the above address
ought to be:

    (set (reg:SI 1 dx [125])
         (plus:SI (mult:SI (reg:SI 1 dx [orig:72 D.78627 ] [72])
                           (const_int 2 [0x2]))
                  (reg:SI 5 di))

Richard

Patch hide | download patch | download mbox

Index: simplify-rtx.c
===================================================================
--- simplify-rtx.c	(revision 191796)
+++ simplify-rtx.c	(working copy)
@@ -5689,6 +5688,21 @@  simplify_subreg (enum machine_mode outermode, rtx
 	return CONST0_RTX (outermode);
     }
 
+  /* Simplify (subreg:SI (plus:DI ((x:DI) (y:DI)), 0)
+     to (plus:SI (subreg:SI (x:DI) 0) (subreg:SI (x:DI) 0)), where
+     the outer subreg is effectively a truncation to the original mode.  */
+  if ((GET_CODE (op) == PLUS
+       || GET_CODE (op) == MINUS)
+      && SCALAR_INT_MODE_P (outermode)
+      && SCALAR_INT_MODE_P (innermode)
+      && GET_MODE_PRECISION (outermode) < GET_MODE_PRECISION (innermode)
+      && subreg_lsb_1 (outermode, innermode, byte) == 0)
+    return simplify_gen_binary (GET_CODE (op), outermode,
+				simplify_gen_subreg (outermode, XEXP (op, 0),
+						     innermode, 0),
+				simplify_gen_subreg (outermode, XEXP (op, 1),
+						     innermode, 0));
+  
   /* Simplify (subreg:QI (lshiftrt:SI (sign_extend:SI (x:QI)) C), 0) into
      to (ashiftrt:QI (x:QI) C), where C is a suitable small constant and
      the outer subreg is effectively a truncation to the original mode.  */