This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: find_reloads_address_1 question



Joern Rennecke wrote:

>> This means that on the first call of find_reloads, the address
>> is changed to
>>    (plus (plus (reg 11) (const_int 124)) (reg 0))
>> which is non-canonical.
>
>This is done assuming that the inner plus is going to be loaded into
>a reload register.
>It would be interesting to find out why this has not happened.

As I said, this is because the address is replaced already on the
*first* call to find_reloads (from within calculate_needs_all_insns)
which has 'replace' set to 0.  However, on the *second* call to
find_reloads (from within reload_as_needed), find_reloads_address
does not go into that special case anymore, because the now modified
address does not fit the pattern the 'if' is testing for anymore ...

Instead the address is passed to find_reloads_address_1 by default,
which then decides to simply reload reg 0.

To fix this, shouldn't the address replacement be done only if
'replace_reloads' is true?

>I think changing REG_OK_FOR_INDEX_P would be preferable if you want to
work
>around this problem in your backend.  You can check reload_in_progress to
>avoid an impact on the register (class) preferences in the pre-reload
phases.

OK, I'll try this.

>A fix in reload at the point that you identified could be to replace the
>index by a pseudo of the same mode before trying checking
>memory_address_p.  In general, the index might be something that is not
>a REG, e.g. a MEM, but then it would make even more sense to use a pseudo
>to consider reloading the entire index into a suitable register.
>There are two places in find_reloads_address where such a change would
have
>to be made, so it would make sense to have a helper function to do the
>check including modifying and restoring the address.

Ah, I see.  Would you consider something like the following patch to be
a proper fix, then?   (Note: I haven't bootstrapped/tested the patch yet,
only verified that it solves my testcase.)

I'm not sure about the replace_reloads parts of the fix.  Maybe the
whole case should not even be entered unless replace_reloads is set?


--- gcc/reload.c    2001/06/12 14:22:49  1.2
+++ gcc/reload.c    2001/10/18 13:27:44
@@ -4581,6 +4581,27 @@
   return tem;
 }

+/* Returns true if AD could be turned into a valid memory reference
+   to mode MODE by reloading the part pointed to by PART into a
+   register.  */
+
+static int
+maybe_memory_address_p (mode, ad, part)
+     enum machine_mode mode;
+     rtx ad;
+     rtx *part;
+{
+  int retv;
+  rtx tem = *part;
+  rtx reg = gen_rtx_REG (GET_MODE (tem), FIRST_PSEUDO_REGISTER);
+
+  *part = reg;
+  retv = memory_address_p (mode, ad);
+  *part = tem;
+
+  return retv;
+}
+
 /* Record all reloads needed for handling memory address AD
    which appears in *LOC in a memory reference to mode MODE
    which itself is found in location  *MEMREFLOC.
@@ -4879,12 +4900,14 @@
            || XEXP (XEXP (ad, 0), 0) == arg_pointer_rtx
 #endif
            || XEXP (XEXP (ad, 0), 0) == stack_pointer_rtx)
-       && ! memory_address_p (mode, ad))
+       && ! maybe_memory_address_p (mode, ad, &XEXP (XEXP (ad, 0), 1)))
     {
-      *loc = ad = gen_rtx_PLUS (GET_MODE (ad),
-                   plus_constant (XEXP (XEXP (ad, 0), 0),
-                               INTVAL (XEXP (ad, 1))),
-                 XEXP (XEXP (ad, 0), 1));
+      ad = gen_rtx_PLUS (GET_MODE (ad),
+               plus_constant (XEXP (XEXP (ad, 0), 0),
+                           INTVAL (XEXP (ad, 1))),
+               XEXP (XEXP (ad, 0), 1));
+      if (replace_reloads)
+    *loc = ad;
       find_reloads_address_part (XEXP (ad, 0), &XEXP (ad, 0),
BASE_REG_CLASS,
                     GET_MODE (ad), opnum, type, ind_levels);
       find_reloads_address_1 (mode, XEXP (ad, 1), 1, &XEXP (ad, 1), opnum,
@@ -4903,12 +4926,14 @@
            || XEXP (XEXP (ad, 0), 1) == arg_pointer_rtx
 #endif
            || XEXP (XEXP (ad, 0), 1) == stack_pointer_rtx)
-       && ! memory_address_p (mode, ad))
+       && ! maybe_memory_address_p (mode, ad, &XEXP (XEXP (ad, 0), 0)))
     {
-      *loc = ad = gen_rtx_PLUS (GET_MODE (ad),
-                   XEXP (XEXP (ad, 0), 0),
-                   plus_constant (XEXP (XEXP (ad, 0), 1),
-                               INTVAL (XEXP (ad, 1))));
+      ad = gen_rtx_PLUS (GET_MODE (ad),
+               XEXP (XEXP (ad, 0), 0),
+               plus_constant (XEXP (XEXP (ad, 0), 1),
+                        INTVAL (XEXP (ad, 1))));
+      if (replace_reloads)
+    *loc = ad;
       find_reloads_address_part (XEXP (ad, 1), &XEXP (ad, 1),
BASE_REG_CLASS,
                     GET_MODE (ad), opnum, type, ind_levels);
       find_reloads_address_1 (mode, XEXP (ad, 0), 1, &XEXP (ad, 0), opnum,



Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]