This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Cleaning up expand optabs code
On 03/22/2011 06:48 PM, Richard Henderson wrote:
> Ok. Watch out for other target problems this week.
This unfortunately broke bootstrap on s390.
An unrecognizable insns is generated:
(insn 22 21 23 4 (set (reg/v:DI 44 [ end])
(mult:DI (sign_extend:DI (mem/s/j:SI (plus:SI (reg/v/f:SI 47 [ foo])
(const_int 4 [0x4])) [0 foo_5(D)->incr_ull+4 S4 A32]))
(sign_extend:DI (subreg:SI (reg:DI 49) 4)))) t.c:12 -1
(nil))
The problem is that expand_binop_directly swaps the operands without rechecking the
predicates afterwards. The old code did:
/* Now, if insn's predicates don't allow our operands, put them into
pseudo regs. */
if (!insn_data[icode].operand[1].predicate (xop0, mode0)
&& mode0 != VOIDmode)
xop0 = copy_to_mode_reg (mode0, xop0);
if (!insn_data[icode].operand[2].predicate (xop1, mode1)
&& mode1 != VOIDmode)
xop1 = copy_to_mode_reg (mode1, xop1);
Right after swapping the operands.
Unfortunately it cannot be simply fixed by swapping the operands in the back end pattern.
Since subreg and reg have different operand precedences. A subreg usually will be second
after a mem while a reg (having same precedence as mem) might be first operand before a mem.
Just copying the pre-patch behaviour fixes the problem for me:
Index: gcc/optabs.c
===================================================================
*** gcc/optabs.c.orig 2011-03-24 12:54:31.000000000 +0100
--- gcc/optabs.c 2011-03-24 12:54:43.000000000 +0100
*************** expand_binop_directly (enum machine_mode
*** 1308,1313 ****
--- 1308,1322 ----
ops[1].value = swap;
}
+ /* Now, if insn's predicates don't allow our operands, put them into
+ pseudo regs. */
+
+ if (!insn_operand_matches (icode, 1, ops[1].value))
+ ops[1].value = copy_to_mode_reg (mode0, ops[1].value);
+
+ if (!insn_operand_matches (icode, 2, ops[2].value))
+ ops[2].value = copy_to_mode_reg (mode1, ops[2].value);
+
pat = GEN_FCN (icode) (ops[0].value, ops[1].value, ops[2].value);
if (pat)
{
Ok?
Bye,
-Andreas-