This is the mail archive of the gcc-patches@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]
Other format: [Raw text]

[PATCH][i386] Fix PR 6585 redundant movl instructions


   In PR 6585 we have a case of poor register allocation because of the
DImode destination in a widening multiply:

	subl	$8, %esp		# 43	pro_epilogue_adjust_stack_1/1	[length = 3]
	movl	%ebx, (%esp)		# 44	*movsi_1/2	[length = 3]
	movl	24(%esp), %ecx		# 34	*movsi_1/1	[length = 4]
	movl	12(%esp), %ebx		# 28	*movsi_1/1	[length = 4]
	movl	%esi, 4(%esp)		# 45	*movsi_1/2	[length = 4]
	movl	16(%esp), %eax		# 35	*movsi_1/1	[length = 4]
	movl	20(%esp), %esi		# 30	*movsi_1/1	[length = 4]
	imull	%ebx, %ecx		# 7	*mulsi3_1/3	[length = 3]
	imull	%esi, %eax		# 8	*mulsi3_1/3	[length = 3]
	addl	%eax, %ecx		# 9	*addsi_1/1	[length = 2]
	movl	%esi, %eax		# 36	*movsi_1/1	[length = 2]
	mull	%ebx			# 10	*umulsidi3_insn	[length = 2]
	movl	(%esp), %ebx		# 48	*movsi_1/1	[length = 3]
	leal	(%ecx,%edx), %esi	# 40	*lea_1		[length = 3]
	movl	%esi, %edx		# 42	*movsi_1/1	[length = 2]
	movl	4(%esp), %esi		# 49	*movsi_1/1	[length = 4]
	addl	$8, %esp		# 50	pro_epilogue_adjust_stack_1/1	[length = 3]
	ret				# 51	return_internal	[length = 1]

   In particular, the final multiplication and addition is asking a lot of
the current register allocator:

(insn:HI 10 9 11 2 /home/rask/pr6585.c:1 (parallel [
            (set (reg:DI 61)
                (mult:DI (zero_extend:DI (reg:SI 66 [ b ]))
                    (zero_extend:DI (reg:SI 64 [ a ]))))
            (clobber (reg:CC 17 flags))
        ]) 304 {*umulsidi3_insn} (expr_list:REG_DEAD (reg:SI 66 [ b ])
        (expr_list:REG_DEAD (reg:SI 64 [ a ])
            (expr_list:REG_UNUSED (reg:CC 17 flags)
                (nil)))))

(insn:HI 12 11 18 2 /home/rask/pr6585.c:1 (parallel [
            (set (subreg:SI (reg:DI 61) 4)
                (plus:SI (reg:SI 62)
                    (subreg:SI (reg:DI 61) 4)))
            (clobber (reg:CC 17 flags))
        ]) 249 {*addsi_1} (expr_list:REG_DEAD (reg:SI 62)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

   It records a conflict between register 61 and 66 and ends up allocating
register 61 to (%esi:%ebx) where we would prefer (%edx:%eax). This patch
fixes it by splitting the DImode destination into two SImode parts before
register allocation, which allows the second subreg lowering pass to replace
the subregs with plain SImode regs which the register allocator can handle.
The result is much better:

	pushl	%ebx			# 40	*pushsi2	[length = 1]
	movl	8(%esp), %ebx		# 28	*movsi_1/1	[length = 4]
	movl	16(%esp), %eax		# 30	*movsi_1/1	[length = 4]
	movl	20(%esp), %ecx		# 37	*movsi_1/1	[length = 4]
	movl	12(%esp), %edx		# 38	*movsi_1/1	[length = 4]
	imull	%ebx, %ecx		# 7	*mulsi3_1/3	[length = 3]
	imull	%eax, %edx		# 8	*mulsi3_1/3	[length = 3]
	addl	%edx, %ecx		# 9	*addsi_1/1	[length = 2]
	mull	%ebx			# 33	*umulsidi3	[length = 2]
	popl	%ebx			# 43	popsi1	[length = 1]
	leal	(%ecx,%edx), %edx	# 39	*lea_1	[length = 3]
	ret				# 44	return_internal	[length = 1]

   Bootstrapped and tested on i686-pc-linux-gnu (all default languages) with
no regressions. Ok for trunk?

2007-12-19  Rask Ingemann Lambertsen  <rask@sygehus.dk>

	PR rtl-optimization/6585
	* config/i386/i386.md ("*umulsidi3_insn"): Split DImode destination
	into SImode parts before register allocation.
	("*mulsidi3_insn"): Likewise.
	("*umulsidi3")("*mulsidi3"): New.

Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	(revision 130944)
+++ gcc/config/i386/i386.md	(working copy)
@@ -7765,13 +7765,41 @@
   "!TARGET_64BIT"
   "")
 
-(define_insn "*umulsidi3_insn"
+(define_insn_and_split "*umulsidi3_insn"
   [(set (match_operand:DI 0 "register_operand" "=A")
 	(mult:DI (zero_extend:DI (match_operand:SI 1 "nonimmediate_operand" "%0"))
 		 (zero_extend:DI (match_operand:SI 2 "nonimmediate_operand" "rm"))))
    (clobber (reg:CC FLAGS_REG))]
   "!TARGET_64BIT
    && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
+  "#"
+  "&& 1"
+  [(parallel
+  [(set (match_dup 3)
+	(subreg:SI (mult:DI (zero_extend:DI (match_dup 1))
+			    (zero_extend:DI (match_dup 2))) 0))
+   (set (match_dup 4)
+	(subreg:SI (mult:DI (zero_extend:DI (match_dup 1))
+			    (zero_extend:DI (match_dup 2))) 4))
+   (clobber (reg:CC FLAGS_REG))]
+  )]
+{
+  operands[3] = simplify_gen_subreg (SImode, operands[0], DImode, 0);
+  operands[4] = simplify_gen_subreg (SImode, operands[0], DImode, 4);
+})
+
+(define_insn "*umulsidi3"
+  [(set (match_operand:SI 0 "register_operand" "=a")
+	(subreg:SI (mult:DI (zero_extend:DI
+			 (match_operand:SI 1 "nonimmediate_operand" "%0"))
+			    (zero_extend:DI
+			 (match_operand:SI 2 "nonimmediate_operand" "rm"))) 0))
+   (set (match_operand:SI 3 "register_operand" "=d")
+	(subreg:SI (mult:DI (zero_extend:DI (match_dup 1))
+			    (zero_extend:DI (match_dup 2))) 4))
+   (clobber (reg:CC FLAGS_REG))]
+  "!TARGET_64BIT
+   && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
   "mul{l}\t%2"
   [(set_attr "type" "imul")
    (set_attr "length_immediate" "0")
@@ -7819,13 +7847,41 @@
   "!TARGET_64BIT"
   "")
 
-(define_insn "*mulsidi3_insn"
+(define_insn_and_split "*mulsidi3_insn"
   [(set (match_operand:DI 0 "register_operand" "=A")
 	(mult:DI (sign_extend:DI (match_operand:SI 1 "nonimmediate_operand" "%0"))
 		 (sign_extend:DI (match_operand:SI 2 "nonimmediate_operand" "rm"))))
    (clobber (reg:CC FLAGS_REG))]
   "!TARGET_64BIT
    && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
+  "#"
+  "&& 1"
+  [(parallel
+  [(set (match_dup 3)
+	(subreg:SI (mult:DI (sign_extend:DI (match_dup 1))
+			    (sign_extend:DI (match_dup 2))) 0))
+   (set (match_dup 4)
+	(subreg:SI (mult:DI (sign_extend:DI (match_dup 1))
+			    (sign_extend:DI (match_dup 2))) 4))
+   (clobber (reg:CC FLAGS_REG))]
+  )]
+{
+  operands[3] = simplify_gen_subreg (SImode, operands[0], DImode, 0);
+  operands[4] = simplify_gen_subreg (SImode, operands[0], DImode, 4);
+})
+
+(define_insn "*mulsidi3"
+  [(set (match_operand:SI 0 "register_operand" "=a")
+	(subreg:SI (mult:DI (sign_extend:DI
+			 (match_operand:SI 1 "nonimmediate_operand" "%0"))
+			    (sign_extend:DI
+			 (match_operand:SI 2 "nonimmediate_operand" "rm"))) 0))
+   (set (match_operand:SI 3 "register_operand" "=d")
+	(subreg:SI (mult:DI (sign_extend:DI (match_dup 1))
+			    (sign_extend:DI (match_dup 2))) 4))
+   (clobber (reg:CC FLAGS_REG))]
+  "!TARGET_64BIT
+   && !(MEM_P (operands[1]) && MEM_P (operands[2]))"
   "imul{l}\t%2"
   [(set_attr "type" "imul")
    (set_attr "length_immediate" "0")

-- 
Rask Ingemann Lambertsen
Danish law requires addresses in e-mail to be logged and stored for a year


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