PATCH [11/n]: Prepare x32: PR rtl-optimization/48155: Reload doesn't handle subreg properly

H.J. Lu hjl.tools@gmail.com
Tue Jun 28 14:24:00 GMT 2011


On Mon, Jun 27, 2011 at 10:03 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> H.J. Lu wrote:
>> On Mon, Jun 27, 2011 at 7:52 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>> > H.J. Lu wrote:
>> >
>> >> Given input:
>> >>
>> >> (plus:SI (subreg:SI (plus:DI (reg/f:DI 7 sp)
>> >> =A0 =A0 =A0 =A0 =A0 =A0 (const_int 16 [0x10])) 0)
>> >> =A0 =A0 (const_int -1 [0xffffffffffffffff]))
>> >
>> > Once again, this seems weird as legitimate address ... =A0If this really
>> > can occur validly, there'll probably need to be an insn+splitter and/or
>> > a secondard reload provided by the back-end to handle it.
>>
>> This is the valid memory address for any instructions which
>> take a memory operand under x32.  How will insn+splitter and/or
>> a secondard reload help x32 here? Do I implement such a thing for
>> all instructions which take a memory operand?
>
> Well, if this *is* already accepted as valid, then I don't understand
> why it is getting reloaded in the first place.
>
>> > With your change below, it seems you're just falling through to
>> > the generic gen_rtx_SET case, right? =A0 How does this help?
>> >
>>
>> I added ix86_simplify_base_disp to i386.c to handle such cases.
>
> I see.  It appears that this routine is used within ix86_decompose_address,
> which means that:
> - addresses containing constructs as above will be accepted as valid
> - the simplification will *not* be done in place in the RTL stream,
>  but on the fly every time the address is looked at
>
> This explains why just emitting a plain SET is accepted.  But if this
> is the case, then the PLUS case in gen_reload should also have worked,
> since the first thing it tries is just a plain SET as well:
>
> [snip]
>         The simplest approach is to try to generate such an insn and see if it
>         is recognized and matches its constraints.  If so, it can be used.
> [snip]
>      insn = emit_insn_if_valid_for_reload (gen_rtx_SET (VOIDmode, out, in));
>      if (insn)
>        return insn;
>
> Can you check in your test case why this isn't accepted here?
>
>
> In general, I'm wondering if it really a good idea to accept
> complex non-simplified expressions like the above as valid addresses,
> instead of simplifying them directly where they are generated, and
> then just accepting the simplified versions.  That simplification
> could occur e.g. in a secondary reload for PLUS (in those cases
> where we actually require a reload), or in a legitimize_reload_address
> handler (in those cases where the address can remain in place).
>

I am testing this patch instead.  It generates better codes.

Thanks.

-- 
H.J.
----
2011-06-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/48155
	* config/i386/i386.md (*lea_0_x32): New.
	* config/i386/predicates.md (pointer_register_operand): Likewise.

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index de6679e..20c22c2 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -5432,6 +5432,34 @@
   [(set_attr "type" "alu")
    (set_attr "mode" "QI")])

+;; Used by reload to support addresses with complex expressions.
+(define_insn_and_split "*lea_0_x32"
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(plus:SI
+	  (subreg:SI
+	    (plus:DI
+	      (match_operand:DI 1 "pointer_register_operand" "r")
+	      (match_operand:DI 2 "immediate_operand" "n")) 0)
+	  (match_operand:SI 3 "immediate_operand" "n")))]
+  "TARGET_X32 && !can_create_pseudo_p ()"
+  "#"
+  "&& reload_completed"
+  [(const_int 0)]
+{
+  rtx pat, src, insn;
+  pat = gen_rtx_PLUS (DImode, operands[1], operands[2]);
+  src = gen_rtx_PLUS (SImode, gen_rtx_SUBREG (SImode, pat, 0),
+		      operands[3]);
+  operands[1] = gen_lowpart (SImode, operands[1]);
+  operands[2] = GEN_INT (INTVAL (operands[2]) + INTVAL (operands[3]));
+  pat = gen_rtx_PLUS (SImode, operands[1], operands[2]);
+  insn = emit_insn (gen_rtx_SET (VOIDmode, operands[0], pat));
+  set_unique_reg_note (insn, REG_EQUIV, src);
+  DONE;
+}
+  [(set_attr "type" "lea")
+   (set_attr "mode" "SI")])
+
 (define_insn "*lea_1"
   [(set (match_operand:P 0 "register_operand" "=r")
 	(match_operand:P 1 "no_seg_address_operand" "p"))]
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 2ceb717..55842c2 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -1219,3 +1219,8 @@
 {
   return !TARGET_X32 || !SYMBOLIC_CONST (op);
 })
+
+;; Test for pointer register operand
+(define_predicate "pointer_register_operand"
+  (and (match_code "reg")
+       (match_test "REG_POINTER (op)")))



More information about the Gcc-patches mailing list