Patchwork [i386] : Fix PR 53228, target attributes in libcpp/lex.c cause illegal instructions to be used elsewhere

login
register
mail settings
Submitter Uros Bizjak
Date May 4, 2012, 4:38 p.m.
Message ID <CAFULd4bSAY-vsnNwTC_nTxnF9VUsqzVbkSA9+pKceAHMWbZ68A@mail.gmail.com>
Download mbox | patch
Permalink /patch/156968/
State New
Headers show

Comments

Uros Bizjak - May 4, 2012, 4:38 p.m.
Hello!

There is a problem with TARGET_CMOVE handling, where in
ix86_option_override_internal, we set TARGET_CMOVE behind option
processing system back. This flag is not cleared during target
attribute handling, and we emit CMOV elsewhere.

Attached patch redefines TARGET_CMOVE as dynamic check, avoiding
re-initialization of ix86_arch_features array.

2012-05-04  Uros Bizjak  <ubizjak@gmail.com>

	PR target/53228
	* config/i386/i386.h (X86_ARCH_CMOV): Rename from X86_ARCH_CMOVE.
	(TARGET_CMOV): Rename from TARGET_CMOVE.
	(TARGET_CMOVE): New define.
	* config/i386/i386.c (ix86_option_override_internal): Use TARGET_CMOV.
	Do not set TARGET_CMOVE here.

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
{,-m32}. I also checked that lex.i test from the PR doesn't generate
unwanted cmov insns.

Committed to mainline SVN, will be backported to all release branches.

Uros.

Patch

Index: i386.c
===================================================================
--- i386.c	(revision 187099)
+++ i386.c	(working copy)
@@ -2190,7 +2190,7 @@  unsigned char ix86_arch_features[X86_ARCH_LAST];
 /* Feature tests against the various architecture variations, used to create
    ix86_arch_features based on the processor mask.  */
 static unsigned int initial_ix86_arch_features[X86_ARCH_LAST] = {
-  /* X86_ARCH_CMOVE: Conditional move was added for pentiumpro.  */
+  /* X86_ARCH_CMOV: Conditional move was added for pentiumpro.  */
   ~(m_386 | m_486 | m_PENT | m_K6),
 
   /* X86_ARCH_CMPXCHG: Compare and exchange was added for 80486.  */
@@ -3504,7 +3504,7 @@  ix86_option_override_internal (bool main_args_p)
 	   -mtune (rather than -march) points us to a processor that has them.
 	   However, the VIA C3 gives a SIGILL, so we only do that for i686 and
 	   higher processors.  */
-	if (TARGET_CMOVE
+	if (TARGET_CMOV
 	    && (processor_alias_table[i].flags & (PTA_PREFETCH_SSE | PTA_SSE)))
 	  x86_prefetch_sse = true;
 	break;
@@ -3780,12 +3780,6 @@  ix86_option_override_internal (bool main_args_p)
       target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
     }
 
-  /* For sane SSE instruction set generation we need fcomi instruction.
-     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.  */
   {
     char *p;
Index: i386.h
===================================================================
--- i386.h	(revision 187099)
+++ i386.h	(working copy)
@@ -430,7 +430,7 @@  extern unsigned char ix86_tune_features[X86_TUNE_L
 
 /* Feature tests against the various architecture variations.  */
 enum ix86_arch_indices {
-  X86_ARCH_CMOVE,		/* || TARGET_SSE */
+  X86_ARCH_CMOV,
   X86_ARCH_CMPXCHG,
   X86_ARCH_CMPXCHG8B,
   X86_ARCH_XADD,
@@ -441,12 +441,17 @@  enum ix86_arch_indices {
 
 extern unsigned char ix86_arch_features[X86_ARCH_LAST];
 
-#define TARGET_CMOVE		ix86_arch_features[X86_ARCH_CMOVE]
+#define TARGET_CMOV		ix86_arch_features[X86_ARCH_CMOV]
 #define TARGET_CMPXCHG		ix86_arch_features[X86_ARCH_CMPXCHG]
 #define TARGET_CMPXCHG8B	ix86_arch_features[X86_ARCH_CMPXCHG8B]
 #define TARGET_XADD		ix86_arch_features[X86_ARCH_XADD]
 #define TARGET_BSWAP		ix86_arch_features[X86_ARCH_BSWAP]
 
+/* For sane SSE instruction set generation we need fcomi instruction.
+   It is safe to enable all CMOVE instructions.  Also, RDRAND intrinsic
+   expands to a sequence that includes conditional move. */
+#define TARGET_CMOVE		(TARGET_CMOV || TARGET_SSE || TARGET_RDRND)
+
 #define TARGET_FISTTP		(TARGET_SSE3 && TARGET_80387)
 
 extern int x86_prefetch_sse;