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

login
register
mail settings
Submitter Uros Bizjak
Date Sept. 27, 2012, 1:04 p.m.
Message ID <CAFULd4aSLiau+Qa9vC_eUsPrRKwvAqYt=uXxGi5Sj5GphCYMmA@mail.gmail.com>
Download mbox | patch
Permalink /patch/187352/
State New
Headers show

Comments

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.
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

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.  */