diff mbox

Restore i?86/x86_64 4.6 and earlier cmov behavior (PR target/54073)

Message ID 20121114230852.GO1886@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 14, 2012, 11:08 p.m. UTC
Hi!

Before Richard Sandiford's 4.7 optab cleanups emit_conditional_move
wasn't testing the predicate on the comparison, and apparently
mov<mode>cc patterns in i386.md can handle some comparisons that the
current predicates on the expanders reject.  This PR on MonteCarlo
kernel shows a hardly predictable floating point UNLT comparison with SImode
conditional increment, on which cmov improves things a lot.

The following patch restores the 4.6 and earlier behavior, if mov<mode>cc
isn't able to handle the comparison, it will FAIL (except for the fallout
discovered during the bootstrap/regtest, that floating point conditional
move didn't check on i686 -m32 for DImode comparisons).

If cmovs are undesirable on likely well predictable conditionals, we should
reject it somewhere else, depending on the predictability of the
conditionals, not based on whether the comparison is ordered or not.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-11-14  Jakub Jelinek  <jakub@redhat.com>

	PR target/54073
	* config/i386/i386.md (mov<mode>cc): Use comparison_operator
	instead of ordered_comparison_operator resp.
	ix86_fp_comparison_operator predicates.
	* config/i386/i386.c (ix86_expand_fp_movcc): Reject TImode
	or for -m32 DImode comparisons.


	Jakub

Comments

Jan Hubicka Nov. 15, 2012, 1:52 a.m. UTC | #1
> Hi!
> 
> Before Richard Sandiford's 4.7 optab cleanups emit_conditional_move
> wasn't testing the predicate on the comparison, and apparently
> mov<mode>cc patterns in i386.md can handle some comparisons that the
> current predicates on the expanders reject.  This PR on MonteCarlo
> kernel shows a hardly predictable floating point UNLT comparison with SImode
> conditional increment, on which cmov improves things a lot.
> 
> The following patch restores the 4.6 and earlier behavior, if mov<mode>cc
> isn't able to handle the comparison, it will FAIL (except for the fallout
> discovered during the bootstrap/regtest, that floating point conditional
> move didn't check on i686 -m32 for DImode comparisons).
> 
> If cmovs are undesirable on likely well predictable conditionals, we should
> reject it somewhere else, depending on the predictability of the
> conditionals, not based on whether the comparison is ordered or not.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2012-11-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/54073
> 	* config/i386/i386.md (mov<mode>cc): Use comparison_operator
> 	instead of ordered_comparison_operator resp.
> 	ix86_fp_comparison_operator predicates.
> 	* config/i386/i386.c (ix86_expand_fp_movcc): Reject TImode
> 	or for -m32 DImode comparisons.

OK, note that I also have patch in testing to do the same for cbranch and
cstore. I however missed the FP cmove variant.

Thanks!
Honza
diff mbox

Patch

--- gcc/config/i386/i386.md.jj	2012-11-13 16:46:04.859106102 +0100
+++ gcc/config/i386/i386.md	2012-11-14 20:31:24.440648957 +0100
@@ -16120,7 +16120,7 @@  (define_peephole2
 
 (define_expand "mov<mode>cc"
   [(set (match_operand:SWIM 0 "register_operand")
-	(if_then_else:SWIM (match_operand 1 "ordered_comparison_operator")
+	(if_then_else:SWIM (match_operand 1 "comparison_operator")
 			   (match_operand:SWIM 2 "<general_operand>")
 			   (match_operand:SWIM 3 "<general_operand>")))]
   ""
@@ -16237,7 +16237,7 @@  (define_split
 (define_expand "mov<mode>cc"
   [(set (match_operand:X87MODEF 0 "register_operand")
 	(if_then_else:X87MODEF
-	  (match_operand 1 "ix86_fp_comparison_operator")
+	  (match_operand 1 "comparison_operator")
 	  (match_operand:X87MODEF 2 "register_operand")
 	  (match_operand:X87MODEF 3 "register_operand")))]
   "(TARGET_80387 && TARGET_CMOVE)
--- gcc/config/i386/i386.c.jj	2012-11-14 20:26:37.000000000 +0100
+++ gcc/config/i386/i386.c	2012-11-14 20:45:54.304190855 +0100
@@ -19847,6 +19847,11 @@  ix86_expand_fp_movcc (rtx operands[])
       return true;
     }
 
+  if (GET_MODE (op0) == TImode
+      || (GET_MODE (op0) == DImode
+	  && !TARGET_64BIT))
+    return false;
+
   /* The floating point conditional move instructions don't directly
      support conditions resulting from a signed integer comparison.  */