Re: i386 longlong.h fixes (Was: ... -masm=intel problems)

[Rask Ingemann Lambertsen <rask at sygehus dot dk>]

On Mon, Oct 22, 2007 at 11:54:10PM +0200, Rask Ingemann Lambertsen wrote:
>    However, if you followed the above, you'll probably have a nasty feeling
> that there are issues with insn canonicalization lurking under the surface,
> and sure enough, everything falls apart when some clod passes in a constant
> for the subtrahend:
>          
> UDWtype test_sub_ddmm (UWtype ah, UWtype al)
> {
>   UWtype reth, retl;
> 
>   sub_ddmmss (reth, retl, ah, al, 1, 1);
>   return (((UDWtype) reth << W_TYPE_SIZE) | retl);
> }
> 
> test_sub_ddmm:
> 	movl	8(%esp), %edx	# 3	*movsi_1/1	[length = 4]
> 	movl	4(%esp), %ecx	# 49	*movsi_1/1	[length = 4]
> 	leal	-1(%edx), %eax	# 50	*lea_1	[length = 3]
> 	subl	$1, %ecx	# 8	*addsi_1/1	[length = 3]
> 	cmpl	%edx, %eax	# 9	*cmpsi_1_insn/1	[length = 2]
> 	seta	%dl	# 10	*setcc_1	[length = 3]
> 	movzbl	%dl, %edx	# 51	*zero_extendqisi2_movzbw	[length = 3]
> 	subl	%edx, %ecx	# 12	*subsi_1/1	[length = 2]
> 	movl	%ecx, %edx	# 45	*movsi_1/1	[length = 2]
> 	ret	# 54	return_internal	[length = 1]

   OK, let's make the testcase easier by using 2 instead of 1 as the
constant:

UDWtype test_sub_ddmm2 (UWtype ah, UWtype al)
{
  UWtype reth, retl;

  sub_ddmmss (reth, retl, ah, al, 2, 2);
  return (((UDWtype) reth << W_TYPE_SIZE) | retl);
}

   This requires just a little tweaking over what we had already

Index: gcc/config/i386/predicates.md
===================================================================
--- gcc/config/i386/predicates.md	(revision 129548)
+++ gcc/config/i386/predicates.md	(working copy)
@@ -1043,3 +1043,16 @@
 
 (define_predicate "absneg_operator"
   (match_code "abs,neg"))
+
+(define_predicate "plus_operator"
+   (and (match_code "plus")
+	(ior (not (match_code "const_int" "1"))
+	     (match_test "INTVAL (XEXP (op, 1)) >= 0"))
+   ))
+
+(define_predicate "minus_operator"
+   (ior (match_code "minus")
+	(and (match_code "plus")
+	     (match_code "const_int" "1")
+	     (match_test "INTVAL (XEXP (op, 1)) < 0"))
+   ))
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 129548)
+++ gcc/config/i386/i386.c	(working copy)
@@ -11313,7 +11313,7 @@ ix86_cc_mode (enum rtx_code code, rtx op
     case GEU:			/* CF=0 */
     case LTU:			/* CF=1 */
       /* Detect overflow checks.  They need just the carry flag.  */
-      if (GET_CODE (op0) == PLUS
+      if (plus_operator (op0, mode)
 	  && rtx_equal_p (op1, XEXP (op0, 0)))
 	return CCCmode;
       else
@@ -11321,7 +11321,7 @@ ix86_cc_mode (enum rtx_code code, rtx op
     case GTU:			/* CF=0 & ZF=0 */
     case LEU:			/* CF=1 | ZF=1 */
       /* Detect overflow checks.  They need just the carry flag.  */
-      if (GET_CODE (op0) == MINUS
+      if (minus_operator (op0, mode)
 	  && rtx_equal_p (op1, XEXP (op0, 0)))
 	return CCCmode;
       else

and using "minus_operator" in the pattern as well

+(define_insn "*sub<mode>3_carry_new"
+  [(set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m,<r>")
+	(match_operator:SWI 4 "minus_operator"
+		[(minus:SWI (match_operand:SWI 1 "nonimmediate_operand" "0,0")
+			    (match_operand:SWI 3 "ix86_carry_flag_operator"))
+		 (match_operand:SWI 2 "<general_operand>" "<r><i>,<r>m")]))
+   (clobber (reg:CC FLAGS_REG))]
+  "ix86_binary_operator_ok (MINUS, <MODE>mode, operands)"
+{
+  if (GET_CODE (operands[4]) == PLUS)
+    operands[2] = GEN_INT (-INTVAL (operands[2]));
+  return "sbb{<imodesuffix>}\t{%2, %0|%0, %2}";
+}
+  [(set_attr "type" "alu")
+   (set_attr "pent_pair" "pu")
+   (set_attr "mode" "<MODE>")])

after which, we do get optimum code in this case also:

test_sub_ddmm2:
	movl	8(%esp), %eax	# 49	*movsi_1/1	[length = 4]
	movl	4(%esp), %edx	# 2	*movsi_1/1	[length = 4]
	addl	$-2, %eax	# 9	*addsi3_cc_overflow/1	[length = 3]
	sbbl	$2, %edx	# 12	*subsi3_carry_new/1	[length = 3]
	ret	# 52	return_internal	[length = 1]

   But the same doesn't happen with 1 as the constant:

test_sub_ddmm1:
	movl	4(%esp), %edx	# 50	*movsi_1/1	[length = 4]
	movl	8(%esp), %eax	# 51	*movsi_1/1	[length = 4]
	subl	$1, %edx	# 8	*addsi_1/1	[length = 3]
	addl	$-1, %eax	# 9	*addsi3_cc_overflow/1	[length = 3]
	sbbl	$0, %edx	# 12	*subsi3_carry_new/1	[length = 3]
	ret	# 54	return_internal	[length = 1]

   The reason is that (plus (minus reg gtu) -1) is simplified to
(plus (not gtu) reg). So the final piece of the puzzle is this:

+; Undo simplification.
+(define_insn_and_split "*sub<mode>3_carry_const1"
+  [(set (match_operand:SWI 0 "nonimmediate_operand")
+	(plus:SWI (not:SWI (match_operand:SWI 2 "ix86_carry_flag_operator"))
+		  (match_operand:SWI 1 "nonimmediate_operand")))
+   (clobber (reg:CC FLAGS_REG))]
+  "(!MEM_P (operands[0]) && !MEM_P (operands[1]))
+   || rtx_equal_p (operands[0], operands[1])"
+  "#"
+  "&& 1"
+  [(parallel
+  [(set (match_dup 0)
+	(plus:SWI (minus:SWI (match_dup 1) (match_dup 2))
+		  (const_int -1)))
+   (clobber (reg:CC FLAGS_REG))]
+  )]
+)

test_sub_ddmm1:
	movl	8(%esp), %eax	# 51	*movsi_1/1	[length = 4]
	movl	4(%esp), %edx	# 52	*movsi_1/1	[length = 4]
	addl	$-1, %eax	# 9	*addsi3_cc_overflow/1	[length = 3]
	sbbl	$1, %edx	# 48	*subsi3_carry_new/1	[length = 3]
	ret	# 55	return_internal	[length = 1]

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