From patchwork Thu Aug  5 08:46:36 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [rtl] : Do not generate insns with mismatched REG_EQUAL mode for
 multiword MULT RTXes.
Date: Wed, 04 Aug 2010 22:46:36 -0000
From: Uros Bizjak <ubizjak@gmail.com>
X-Patchwork-Id: 60937
Message-Id: <AANLkTimyp1v2zfhPTd5vdY24gm_AgopgO+O8TooFHaSz@mail.gmail.com>
To: Steven Bosscher <stevenb.gcc@gmail.com>
Cc: gcc-patches@gcc.gnu.org

On Thu, Aug 5, 2010 at 1:09 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Thu, Aug 5, 2010 at 12:20 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> Perhaps we should generate a new insn that would survive optimization passes?
>
> I think you should just take a new register, instead of generating a
> no-op move. This probably helps for other things as well, e.g. for
> passes that look at DF_REG_DEF_COUNT (reg_scan, alias, ...). OTOH,
> maybe that only "helps" up to CPROP pass, if the register is
> copy-propagated and the set is made useless.  Dunno...

Thanks for your comments! Following your suggestion, I propose
attached patch, where we expand shift into temporary and copy this
temporary to final target (the same way other algorithms do via
force_operand). The note now attaches to non-noop insn:

;; D.2028_13 = acc_24 * 10;

(insn 51 50 52 920501-6.c:17 (set (reg:DI 107)
        (reg/v:DI 87 [ acc ])) -1 (nil))

(insn 52 51 53 920501-6.c:17 (set (reg:SI 109)
        (lshiftrt:SI (subreg:SI (reg:DI 107) 4)
            (const_int 31 [0x1f]))) -1 (nil))

(insn 53 52 54 920501-6.c:17 (set (subreg:SI (reg:DI 108) 0)
        (ashift:SI (subreg:SI (reg:DI 107) 0)
            (const_int 1 [0x1]))) -1 (nil))

(insn 54 53 55 920501-6.c:17 (set (subreg:SI (reg:DI 108) 0)
        (ior:SI (reg:SI 109)
            (subreg:SI (reg:DI 108) 0))) -1 (nil))

(insn 55 54 56 920501-6.c:17 (set (subreg:SI (reg:DI 108) 4)
        (ashift:SI (subreg:SI (reg:DI 107) 4)
            (const_int 1 [0x1]))) -1 (nil))

(insn 56 55 57 920501-6.c:17 (set (reg:DI 107)
        (reg:DI 108)) -1 (expr_list:REG_EQUAL (mult:DI (reg/v:DI 87 [ acc ])
            (const_int 2 [0x2]))
        (nil)))

When this stream is processed by subreg1 pass, the register is split
into SImode regs, and REG_EQUAL note is *correctly* removed.

OTOH, without the patch, REG_EQUAL survives through subreg1 pass,
leaving following semi-invalid RTX:

(insn 30 29 31 4 920501-6.c:17 (set (reg:SI 225 [+4 ])
        (ashift:SI (reg:SI 223 [+4 ])
            (const_int 1 [0x1]))) 41 {ashlsi3} (expr_list:REG_EQUAL
(mult:DI (reg/v:DI 41 [ s ])
            (const_int 2 [0x2]))
        (nil)))

Also, looking through the dumps, I saw:

920501-6.c.152r.fwprop1: Setting REG_EQUAL note
920501-6.c.152r.fwprop1: Setting REG_EQUAL note
920501-6.c.152r.fwprop1: Setting REG_EQUAL note

So, it looks to me, that somebody actually thought on copying
REG_EQUALs when values are propagated...

2010-08-04  Uros Bizjak  <ubizjak@gmail.com>

	* expmed.c (expand_mult_const) <case alg_shift:>: Expand shift into
	temporary. Emit move from temporary to accum, so REG_EQUAL note will
	be attached to this insn in correct mode.

This patch fixes original ivopts failure as well.

Patch is currently regression testing on x86_64-pc-linux-gnu. OK for
mainline and release branches, if it passes testing?

Uros.


Index: expmed.c
===================================================================
--- expmed.c	(revision 162856)
+++ expmed.c	(working copy)
@@ -3006,9 +3006,10 @@
       switch (alg->op[opno])
 	{
 	case alg_shift:
-	  accum = expand_shift (LSHIFT_EXPR, mode, accum,
-				build_int_cst (NULL_TREE, log),
-				NULL_RTX, 0);
+	  tem = expand_shift (LSHIFT_EXPR, mode, accum,
+			      build_int_cst (NULL_TREE, log),
+			      NULL_RTX, 0);
+	  emit_move_insn (accum, tem);
 	  val_so_far <<= log;
 	  break;