Re: [PATCH, i386]: Fix __builtin_ia32_rdrand*_step expansion

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, May 19, 2011 at 12:32 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
> Attached patch fixes ICE with -m32 -march=i386 -mrdrand on freebsd
> [1]. The problem was that __builtin_ia32_rdrand*_step expands to a
> sequence that includes conditional move, so we should set
> TARGET_CMOVE. Additionally, the patch fixes expansion with void
> return:
>
> void
> foo (unsigned short *x)
> {
>  _rdrand16_step (x);
> }
>
> And finally, move to the memory was moved closer to the producer, to
> eventually generate few live registers.
>
> 2011-05-19  Uros Bizjak  <ubizjak@gmail.com>
>
>        * config/i386/i386.c (option_override_internal): Enable TARGET_CMOVE
>        when TARGET_RDRND is active.
>        (ix86_expand_builtin) <case IX86_BUILTIN_RDRAND*_STEP>: Generate
>        dummy SImode target register when target is NULL.
>
> Patch was bootstrapped and tested on x86_64-pc-linux-gnu, will be
> committed to SVN mainline and 4.6.
>

I backported it to ix86/gcc-4_5-branch branch.

Thanks.

-- 
H.J.
---
diff --git a/gcc/ChangeLog.ix86 b/gcc/ChangeLog.ix86
index 6d56dee..ba849e3 100644
--- a/gcc/ChangeLog.ix86
+++ b/gcc/ChangeLog.ix86
@@ -1,3 +1,13 @@
+2011-05-19  H.J. Lu  <hongjiu.lu@intel.com>
+
+	Backport from mainline
+	2011-05-19  Uros Bizjak  <ubizjak@gmail.com>
+
+	* config/i386/i386.c (option_override_internal): Enable TARGET_CMOVE
+	when TARGET_RDRND is active.
+	(ix86_expand_builtin) <case IX86_BUILTIN_RDRAND{16,32,64}_STEP>:
+	Generate dummy SImode target register when target is NULL.
+
 2010-12-28  H.J. Lu  <hongjiu.lu@intel.com>

 	Backport from mainline
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 13f2e11..9fccc8c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3471,8 +3471,9 @@ override_options (bool main_args_p)
     }

   /* For sane SSE instruction set generation we need fcomi instruction.
-     It is safe to enable all CMOVE instructions.  */
-  if (TARGET_SSE)
+     It is safe to enable all CMOVE instructions.  Also, RDRAND intrinsic
+     expands to a sequence that includes conditional move. */
+  if (TARGET_SSE || TARGET_RDRND)
     TARGET_CMOVE = 1;

   /* Figure out what ASM_GENERATE_INTERNAL_LABEL builds as a prefix.  */
@@ -24540,6 +24541,12 @@ rdrand_step:
       op0 = gen_reg_rtx (mode0);
       emit_insn (GEN_FCN (icode) (op0));

+      arg0 = CALL_EXPR_ARG (exp, 0);
+      op1 = expand_normal (arg0);
+      if (!address_operand (op1, VOIDmode))
+	op1 = copy_addr_to_reg (op1);
+      emit_move_insn (gen_rtx_MEM (mode0, op1), op0);
+
       op1 = gen_reg_rtx (SImode);
       emit_move_insn (op1, CONST1_RTX (SImode));

@@ -24554,17 +24561,13 @@ rdrand_step:
       else
 	op2 = gen_rtx_SUBREG (SImode, op0, 0);

+      if (target == 0)
+	target = gen_reg_rtx (SImode);
+
       pat = gen_rtx_GEU (VOIDmode, gen_rtx_REG (CCCmode, FLAGS_REG),
 			 const0_rtx);
-      emit_insn (gen_rtx_SET (VOIDmode, op1,
+      emit_insn (gen_rtx_SET (VOIDmode, target,
 			      gen_rtx_IF_THEN_ELSE (SImode, pat, op2, op1)));
-      emit_move_insn (target, op1);
-
-      arg0 = CALL_EXPR_ARG (exp, 0);
-      op1 = expand_normal (arg0);
-      if (!address_operand (op1, VOIDmode))
-	op1 = copy_addr_to_reg (op1);
-      emit_move_insn (gen_rtx_MEM (mode0, op1), op0);
       return target;

     default: