This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Bug in cse_insn() and emit_libcall_block()
- To: gcc at gcc dot gnu dot org
- Subject: Bug in cse_insn() and emit_libcall_block()
- From: Stephane Carrez <Stephane dot Carrez at worldnet dot fr>
- Date: Tue, 27 Feb 2001 23:12:37 +0100
Hi!
I found a problem that occurs when several library calls returning
their value in a stack slot are made. At some point, the CSE pass
incorrectly deletes the copy that is made after the libcall. This
copy is used to save the stack slot content into a register.
When such library call is generated the pseudo rtl is:
(push arguments)
(push (reg/f virtual-stack-vars))
(call_insn "_libcall")
(set (reg M) (mem (reg/f virtual-stack-vars))
...
<second call>
(set (reg N) (mem (reg/f virtual-stack-vars))
...
<third call>
(set (reg O) (mem (reg/f virtual-stack-vars))
...
<fourth call>
(set (reg P) (mem (reg/f virtual-stack-vars))
During the first CSE pass, the first 'set' to save the stack slot is kept.
It is identified as register candidate for cse. Then, the next 'set's
have their source operand (mem (reg/f virtual-stack-vars)) replaced
by the first register, that is (reg M) in the above pseudo-rtl. That is,
the pseudo rtl becomes:
<first call>
(set (reg M) (mem (reg/f virtual-stack-vars))
...
<second call>
(set (reg N) (reg M))
...
<third call>
(set (reg O) (reg M))
...
<fourth call>
(set (reg P) (reg M))
This is not good because the stack slot is modified by the function call,
and the role of the 'set' instruction after it, is to get a copy in a safe
place.
I fixed the problem as follows and would like to know your opinion about that.
- In cse_insn(), cse.c:5074, there is a check for a REG_EQUIV note that could
be present in the insn. As far as I understand, the purpose of this test
is to detect the above 'set' instruction that copy the stack slot
to a register. In that case, the 'set' is marked as src_volatile, which
disables the cse replacement.
/* If SRC is a MEM, there is a REG_EQUIV note for SRC, and DEST is
a pseudo that is set more than once, do not record SRC. Using
SRC as a replacement for anything else will be incorrect in that
situation. Note that this usually occurs only for stack slots,
in which case all the RTL would be referring to SRC, so we don't
lose any optimization opportunities by not having SRC in the
hash table. */
if (GET_CODE (src) == MEM
&& find_reg_note (insn, REG_EQUIV, src) != 0
&& GET_CODE (dest) == REG
&& REGNO (dest) >= FIRST_PSEUDO_REGISTER
&& REG_N_SETS (REGNO (dest)) != 1)
sets[i].src_volatile = 1;
- In emit_libcall_block(), optabs.c:2799, no REG_EQUIV note is emitted when
a stack slot is used. I modified that function to generate a REG_EQUIV
note when the result is a MEM (instead of REG in general).
- I had to change a little bit the test in cse.c:
o In the REG_EQUIV note, the rtx pointer is different than the rtx
pointer of the source. The two expressions are identical (rtx_equal_p),
but at some place, a copy_rtx is probably made (the function was
inlined so this may be the reason). Since find_reg_note() checks
for pointer equality, it is not enough.
o The test REG_N_SETS (REGNO (dest)) != 1 always fails because
there is always one set for these registers.
The test becomes:
if (GET_CODE (src) == MEM
&& (tem = find_reg_note (insn, REG_EQUIV, 0))
&& GET_CODE (dest) == REG
&& REGNO (dest) >= FIRST_PSEUDO_REGISTER
&& rtx_equal_p (XEXP (tem, 0), src))
sets[i].src_volatile = 1;
After these changes, a REG_EQUIV note is attached to the 'set' instructions
and it is recognized by cse. The set instructions are no longer modified
and eliminated.
I would like to know if the generation of REG_EQUIV note for this kind
of problem is the good way to go, and if fixing in that direction would be
acceptable.
Thanks,
Stephane
Below are 3 real extract of rtl (from 68HC11 target), 2 before the fix
and the last one with the fix.
---------- rtl.02.jump (before CSE)
(call_insn 342 __muldi3 ...)
(insn/i 344 343 345 (parallel[
(set (reg:DI 123)
(mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 336 (expr_list:REG_EQUAL (mult:DI (reg:DI 120)
(reg:DI 121))
(nil))))
(...)
(call_insn 352 __muldi3 ...)
(insn/i 354 353 355 (parallel[
(set (reg:DI 127)
(mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 349 (expr_list:REG_EQUAL (mult:DI (reg:DI 125)
(reg:DI 126))
(nil))))
(...)
(call_insn 362 __muldi3 ...)
(insn/i 364 363 365 (parallel[
(set (reg:DI 131)
(mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 359 (expr_list:REG_EQUAL (mult:DI (reg:DI 129)
(reg:DI 130))
(nil))))
----------- rtl.03.cse (after cse pass)
(call_insn 342 __muldi3 ...)
(insn/i 344 343 345 (parallel[
(set (reg:DI 123)
(mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 336 (expr_list:REG_EQUAL (mult:DI (reg:DI 120)
(reg:DI 121))
(nil))))
(...)
(call_insn 352 __muldi3 ...)
(insn/i 354 353 355 (parallel[
(set (reg:DI 127)
(reg:DI 123))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 349 (expr_list:REG_EQUAL (mult:DI (reg:DI 125)
(reg:DI 121))
(nil))))
(...)
(call_insn 362 __muldi3 ...)
(insn/i 364 363 365 (parallel[
(set (reg:DI 131)
(reg:DI 123))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 359 (expr_list:REG_EQUAL (mult:DI (reg:DI 120)
(reg:DI 130))
(nil))))
---------- rtl.03.cse with REG_EQUIV fix
...
(call_insn 342 ...)
(insn/i 344 343 345 (parallel[
(set (reg:DI 123)
(mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 336 (expr_list:REG_EQUIV (mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22)
(expr_list:REG_EQUAL (mult:DI (reg:DI 120)
(reg:DI 121))
(nil)))))
...
(call_insn 352 ...)
(insn/i 354 353 355 (parallel[
(set (reg:DI 127)
(mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22))
(clobber (scratch:HI))
] ) 10 {movdi} (nil)
(insn_list:REG_RETVAL 349 (expr_list:REG_EQUIV (mem/f:DI (plus:HI (reg/f:HI 46 *sframe)
(const_int 61 [0x3d])) 22)
(expr_list:REG_EQUAL (mult:DI (reg:DI 125)
(reg:DI 121))
(nil)))))