Patchwork PATCH [6/n] X32: Supprot 32bit address

login
register
mail settings
Submitter Uros Bizjak
Date July 19, 2011, 5:33 p.m.
Message ID <CAFULd4ZhBwev_jtQ-ra4o_y+upeVBZ4pD7L40Sc_ZnCs-A5zdg@mail.gmail.com>
Download mbox | patch
Permalink /patch/105499/
State New
Headers show

Comments

Uros Bizjak - July 19, 2011, 5:33 p.m.
On Tue, Jul 19, 2011 at 6:37 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> Sometimes, the compiler is really creative in inventing instructions:
>>>
>>> (insn 47 46 49 7 (set (reg:SI 68 [ D.1686 ])
>>>         (subreg:SI (plus:SF (reg:SF 159 [ D.1685 ])
>>>                 (reg:SF 159 [ D.1685 ])) 0)) omp_atomic1.f90:17 247 {*lea_2}
>>>      (expr_list:REG_DEAD (reg:SF 159 [ D.1685 ])
>>>         (nil)))
>>>
>>> Really funny.
>>
>> That's the job of combiner to try all kinds of stuff and it is the
>> responsibility of the backend to reject those.  I think it would be better
>> to get back to testing Pmode in the legitimate address hook, perhaps
>> allowing ptr_mode too in addition to Pmode (which for -m32/-m64 won't mean
>> any change, just for -mx32).
>
> Actually, there is a bypass in ix86_decompose_address, and this RTX
> squeezed through. IMO constructs like this should be rejected in
> i_d_a, which effectively only moves Pmode/ptr_mode check here.
>
> I'm looking into it.

The problem was in fact the declaration of no_seg_address_operand
predicate that was defined as special predicate and this way ignoring
the mode of the operand.

Attached patch also includes check for DImode SUBREGS for base
register, to eventually save x32 some trouble in future.

I'm currently regression testing the patch added to the patch that
removed Pmode checks.

H.J., can you please test it on x32?

Uros.
Uros Bizjak - July 19, 2011, 5:35 p.m.
On Tue, Jul 19, 2011 at 7:33 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>> Sometimes, the compiler is really creative in inventing instructions:
>>>>
>>>> (insn 47 46 49 7 (set (reg:SI 68 [ D.1686 ])
>>>>         (subreg:SI (plus:SF (reg:SF 159 [ D.1685 ])
>>>>                 (reg:SF 159 [ D.1685 ])) 0)) omp_atomic1.f90:17 247 {*lea_2}
>>>>      (expr_list:REG_DEAD (reg:SF 159 [ D.1685 ])
>>>>         (nil)))
>>>>
>>>> Really funny.
>>>
>>> That's the job of combiner to try all kinds of stuff and it is the
>>> responsibility of the backend to reject those.  I think it would be better
>>> to get back to testing Pmode in the legitimate address hook, perhaps
>>> allowing ptr_mode too in addition to Pmode (which for -m32/-m64 won't mean
>>> any change, just for -mx32).
>>
>> Actually, there is a bypass in ix86_decompose_address, and this RTX
>> squeezed through. IMO constructs like this should be rejected in
>> i_d_a, which effectively only moves Pmode/ptr_mode check here.
>>
>> I'm looking into it.
>
> The problem was in fact the declaration of no_seg_address_operand
> predicate that was defined as special predicate and this way ignoring
> the mode of the operand.

This change should be backported to 4.6 and 4.5.

Uros.
H.J. Lu - July 19, 2011, 5:49 p.m.
On Tue, Jul 19, 2011 at 10:33 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Tue, Jul 19, 2011 at 6:37 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>> Sometimes, the compiler is really creative in inventing instructions:
>>>>
>>>> (insn 47 46 49 7 (set (reg:SI 68 [ D.1686 ])
>>>>         (subreg:SI (plus:SF (reg:SF 159 [ D.1685 ])
>>>>                 (reg:SF 159 [ D.1685 ])) 0)) omp_atomic1.f90:17 247 {*lea_2}
>>>>      (expr_list:REG_DEAD (reg:SF 159 [ D.1685 ])
>>>>         (nil)))
>>>>
>>>> Really funny.
>>>
>>> That's the job of combiner to try all kinds of stuff and it is the
>>> responsibility of the backend to reject those.  I think it would be better
>>> to get back to testing Pmode in the legitimate address hook, perhaps
>>> allowing ptr_mode too in addition to Pmode (which for -m32/-m64 won't mean
>>> any change, just for -mx32).
>>
>> Actually, there is a bypass in ix86_decompose_address, and this RTX
>> squeezed through. IMO constructs like this should be rejected in
>> i_d_a, which effectively only moves Pmode/ptr_mode check here.
>>
>> I'm looking into it.
>
> The problem was in fact the declaration of no_seg_address_operand
> predicate that was defined as special predicate and this way ignoring
> the mode of the operand.
>
> Attached patch also includes check for DImode SUBREGS for base
> register, to eventually save x32 some trouble in future.
>
> I'm currently regression testing the patch added to the patch that
> removed Pmode checks.
>
> H.J., can you please test it on x32?
>

Doing it now.

Thanks.
H.J. Lu - July 19, 2011, 6:41 p.m.
On Tue, Jul 19, 2011 at 10:49 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jul 19, 2011 at 10:33 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Tue, Jul 19, 2011 at 6:37 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>> Sometimes, the compiler is really creative in inventing instructions:
>>>>>
>>>>> (insn 47 46 49 7 (set (reg:SI 68 [ D.1686 ])
>>>>>         (subreg:SI (plus:SF (reg:SF 159 [ D.1685 ])
>>>>>                 (reg:SF 159 [ D.1685 ])) 0)) omp_atomic1.f90:17 247 {*lea_2}
>>>>>      (expr_list:REG_DEAD (reg:SF 159 [ D.1685 ])
>>>>>         (nil)))
>>>>>
>>>>> Really funny.
>>>>
>>>> That's the job of combiner to try all kinds of stuff and it is the
>>>> responsibility of the backend to reject those.  I think it would be better
>>>> to get back to testing Pmode in the legitimate address hook, perhaps
>>>> allowing ptr_mode too in addition to Pmode (which for -m32/-m64 won't mean
>>>> any change, just for -mx32).
>>>
>>> Actually, there is a bypass in ix86_decompose_address, and this RTX
>>> squeezed through. IMO constructs like this should be rejected in
>>> i_d_a, which effectively only moves Pmode/ptr_mode check here.
>>>
>>> I'm looking into it.
>>
>> The problem was in fact the declaration of no_seg_address_operand
>> predicate that was defined as special predicate and this way ignoring
>> the mode of the operand.
>>
>> Attached patch also includes check for DImode SUBREGS for base
>> register, to eventually save x32 some trouble in future.
>>
>> I'm currently regression testing the patch added to the patch that
>> removed Pmode checks.
>>
>> H.J., can you please test it on x32?
>>
>
> Doing it now.
>

No regressions in GCC testsuite on x32.

Thanks

Patch

Index: predicates.md
===================================================================
--- predicates.md	(revision 176462)
+++ predicates.md	(working copy)
@@ -796,7 +796,7 @@ 
 
 ;; Return true if op if a valid address, and does not contain
 ;; a segment override.
-(define_special_predicate "no_seg_address_operand"
+(define_predicate "no_seg_address_operand"
   (match_operand 0 "address_operand")
 {
   struct ix86_address parts;
Index: i386.c
===================================================================
--- i386.c	(revision 176462)
+++ i386.c	(working copy)
@@ -11085,8 +11085,16 @@  ix86_decompose_address (rtx addr, struct
   int retval = 1;
   enum ix86_address_seg seg = SEG_DEFAULT;
 
-  if (REG_P (addr) || GET_CODE (addr) == SUBREG)
+  if (REG_P (addr))
     base = addr;
+  else if (GET_CODE (addr) == SUBREG)
+    {
+      /* Allow only subregs of DImode hard regs.  */
+      if (register_no_elim_operand (SUBREG_REG (addr), DImode))
+	base = addr;
+      else
+	return 0;
+    }
   else if (GET_CODE (addr) == PLUS)
     {
       rtx addends[4], op;