PATCH bug with synthesized movdi: need transfer reload replacements to parts.
Hans-Peter Nilsson
hans-peter.nilsson@axis.com
Mon Apr 3 11:05:00 GMT 2000
It might be known (I think I've seen unspecified advice
somewhere) that a 32-bit port that wants to implement 64-bit
stuff (long long) is better off defining a movdi insn; if for no
other reason than that there are bugs lurking. Here's a fix for
one; I know of at least another. (Patch known, analysis to write.)
Sorry, I don't have a specific test-case for an existing target
for this. Depending on the target, gcc.c-torture/execute/920501-2.c
or gcc.c-torture/execute/991216-1.c might emit bad code for -O0.
Higher optimizations tend to sweep up the bad "left-over" parts.
For ports in CVS, it seems only the fr30 port is a potential
victim; all other live ports seem to have movdi patterns.
If we can't get to a commit-or-not-conclusion based on analysis
only, this might have to wait until the CRIS port is submitted
some time in the next few weeks.
The target description has a real cmpdi insn, but the movdi is
left to GCC to synthesize (which really *should* work):
(define_insn "cmpdi"
[(set (cc0)
(compare (match_operand:DI 0 "nonimmediate_operand" "r,r,r,r,r,r,o")
(match_operand:DI 1 "general_operand" "K,I,P,n,r,o,r")))]
""
"...")
For gcc.c-torture/execute/920501-2.c, a cmpdi RTX needs global
reloading:
(insn 185 326 186 (set (cc0)
(compare (mem/f:DI (reg/v:SI 22) 0)
(const_int 1 [0x1]))) 4 {cmpdi} (nil)
(nil))
Now, pseudo-reg 22 gets a stack slot, and the mem is reloaded
out of the instruction into a register, since the preferred
reload is alternative 1; "K" matches constants 0..31.
Reg 22 is replaced with reg 10, and the mem move is moved out and
synthesized in SImode parts.
Bad things happen with the synthesis in gen_reload ->
gen_move_insn -> emit_move_insn_1; the location where reg 22 is
replaced with reg 10 is changed into two new RTX:es, both still
having reg 22; the recorded replacement is lost.
Later on, the "left-over" reg 22 is recognized as having an
equivalent in the stack-frame, which is substituted straight in,
and we end up with an unrecognizable insn (r8 is frame-pointer):
(insn 325 324 326 (set (reg:SI 9 r9)
(mem/f:SI (mem:SI (plus:SI (reg:SI 8 r8)
(const_int -84 [0xffffffac])) 0) 0)) -1 (nil)
(nil))
This is the low part of the abnormal reloaded-for-input "movdi".
I did not find a suitable function to use for adjusting the
replacements, so I wrote copy_mem_subword_replacements to be
just enough what I needed. The function that seemed closest,
copy_replacements, does not support that the "copy" can have
another form, in this case having a "(plus reg offset)" where
the original just had "reg".
This patch was bootstrapped on sparc-sun-solaris2.6 and
bootstrapped and checked with no additional failures on
alpha-unknown-linux-gnu and i686-pc-linux-gnu.
Is this analysis understandable and enough;
is this ok to install?
Mon Apr 3 19:24:40 2000 Hans-Peter Nilsson <hp@axis.com>
* expr.c (emit_move_insn_1): Copy reload replacements
for multi-word MEM moves if reload_in_progress.
* reload.c (copy_mem_subword_replacements): New.
* rtl.h (copy_mem_subword_replacements): Declare.
Index: rtl.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/rtl.h,v
retrieving revision 1.183
diff -c -p -r1.183 rtl.h
*** rtl.h 2000/03/31 16:24:30 1.183
--- rtl.h 2000/04/03 17:23:37
*************** extern void purge_addressof PARAMS ((
*** 1584,1589 ****
--- 1584,1590 ----
/* In reload.c */
extern int operands_match_p PARAMS ((rtx, rtx));
extern int safe_from_earlyclobber PARAMS ((rtx, rtx));
+ extern void copy_mem_subword_replacements PARAMS ((rtx, rtx));
/* In stmt.c */
extern void set_file_and_line_for_stmt PARAMS ((const char *, int));
Index: reload.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload.c,v
retrieving revision 1.102
diff -c -p -r1.102 reload.c
*** reload.c 2000/03/25 18:34:03 1.102
--- reload.c 2000/04/03 17:23:39
*************** find_reloads_subreg_address (x, force_re
*** 5523,5529 ****
/* Substitute into the current INSN the registers into which we have reloaded
the things that need reloading. The array `replacements'
! says contains the locations of all pointers that must be changed
and says what to replace them with.
Return the rtx that X translates into; usually X, but modified. */
--- 5523,5529 ----
/* Substitute into the current INSN the registers into which we have reloaded
the things that need reloading. The array `replacements'
! contains the locations of all pointers that must be changed
and says what to replace them with.
Return the rtx that X translates into; usually X, but modified. */
*************** find_replacement (loc)
*** 5681,5686 ****
--- 5681,5739 ----
}
return *loc;
+ }
+
+ /* Similar to copy_replacements, but used when the new rtx is a subword of
+ the original. */
+
+ void
+ copy_mem_subword_replacements (x, y)
+ rtx x;
+ rtx y;
+ {
+ int j;
+ enum rtx_code code = GET_CODE (x);
+ struct replacement *r;
+ rtx *where;
+
+ /* We currently handle only the case when x is a (MEM REG). We also
+ make an assumption that subwords of MEM are formed as (MEM (PLUS REG
+ offset)) when offset is nonzero. */
+ if (code == MEM)
+ {
+ /* We may also need to handle when the old MEM was a (PLUS REG ...),
+ but that code is not yet written. */
+ if (GET_CODE (XEXP (x, 0)) != REG)
+ {
+ /* If there's no replacement for that item, just return; do not
+ abort. */
+ if (find_replacement (&XEXP (x, 0)) == XEXP (x, 0))
+ return;
+
+ abort ();
+ }
+
+ if (GET_CODE (XEXP (y, 0)) == PLUS)
+ where = &XEXP (XEXP (y, 0), 0);
+ else
+ where = &XEXP (y, 0);
+ }
+ else
+ abort ();
+
+ for (j = 0; j < n_replacements; j++)
+ {
+ /* We need not check replacements[j].subreg_loc since we do not
+ allow SUBREGs as input. */
+ if (replacements[j].where == &XEXP (x, 0))
+ {
+ r = &replacements[n_replacements++];
+ r->where = where;
+ r->subreg_loc = 0;
+ r->what = replacements[j].what;
+ r->mode = replacements[j].mode;
+ }
+ }
}
/* Return nonzero if register in range [REGNO, ENDREGNO)
Index: expr.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/expr.c,v
retrieving revision 1.228
diff -c -p -r1.228 expr.c
*** expr.c 2000/04/01 00:09:21 1.228
--- expr.c 2000/04/03 17:23:40
*************** emit_move_insn_1 (x, y)
*** 2760,2765 ****
--- 2760,2775 ----
need_clobber |= (GET_CODE (xpart) == SUBREG);
+ if (reload_in_progress)
+ {
+ /* We need to make sure that any pending replacements are
+ carried over and suitably adjusted for the subword. */
+ if (GET_CODE (x) == MEM)
+ copy_mem_subword_replacements (x, xpart);
+ if (GET_CODE (y) == MEM)
+ copy_mem_subword_replacements (y, ypart);
+ }
+
last_insn = emit_move_insn (xpart, ypart);
}
brgds, H-P
More information about the Gcc-patches
mailing list