[PATCH] D68255: [X86] Remove AVX/AVX512 check from validateOperandSize, just always accept 512

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: echristo, rnk, efriedma, RKSimon, spatel, erichkeane.

The AVX/AVX512 check were based on the command line features. But the function may have a target attribute that gives more features than the command line. We shouldn't fail for 512 operand if the command line doesn't support it, but the target attribute does. gcc supports this.

I don't know how to get to the target attribute here, or if we even can without creating a layering issue. So I've just removed the check entirely and always allow 512 bits. In some simple testing, the backend seems to produce an error like "couldn't allocate output register for constraint 'x'" when the type is larger than supported by the subtarget features. So I think we can handle this gracefully without crashing.


https://reviews.llvm.org/D68255

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/x86_32-inline-asm.c
  clang/test/CodeGen/x86_64-PR42672.c


Index: clang/test/CodeGen/x86_64-PR42672.c
===================================================================
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -100,7 +100,7 @@
 #endif
 }
 
-// CHECK-IMPOSSIBLE_X: invalid output size for constraint
+// CHECK-IMPOSSIBLE_X: impossible constraint in asm: can't store value into a register
 
 // http://crbug.com/999160
 // Clang used to report the following message:
Index: clang/test/CodeGen/x86_32-inline-asm.c
===================================================================
--- clang/test/CodeGen/x86_32-inline-asm.c
+++ clang/test/CodeGen/x86_32-inline-asm.c
@@ -46,7 +46,7 @@
   __asm__ volatile("foo1 %0" : : "f" (val256)); // expected-error {{invalid input size for constraint 'f'}}
   __asm__ volatile("foo1 %0" : : "t" (val256)); // expected-error {{invalid input size for constraint 't'}}
   __asm__ volatile("foo1 %0" : : "u" (val256)); // expected-error {{invalid input size for constraint 'u'}}
-  __asm__ volatile("foo1 %0" : : "x" (val512)); // expected-error {{invalid input size for constraint 'x'}}
+  __asm__ volatile("foo1 %0" : : "x" (val512)); // No error.
 
   __asm__ volatile("foo1 %0" : "=R" (val)); // expected-error {{invalid output size for constraint '=R'}}
   __asm__ volatile("foo1 %0" : "=q" (val)); // expected-error {{invalid output size for constraint '=q'}}
@@ -60,13 +60,8 @@
   __asm__ volatile("foo1 %0" : "=A" (val128)); // expected-error {{invalid output size for constraint '=A'}}
   __asm__ volatile("foo1 %0" : "=t" (val256)); // expected-error {{invalid output size for constraint '=t'}}
   __asm__ volatile("foo1 %0" : "=u" (val256)); // expected-error {{invalid output size for constraint '=u'}}
-  __asm__ volatile("foo1 %0" : "=x" (val512)); // expected-error {{invalid output size for constraint '=x'}}
+  __asm__ volatile("foo1 %0" : "=x" (val512)); // No error.
 
-#ifdef __AVX__
   __asm__ volatile("foo1 %0" : : "x" (val256)); // No error.
   __asm__ volatile("foo1 %0" : "=x" (val256));  // No error.
-#else
-  __asm__ volatile("foo1 %0" : : "x" (val256)); // expected-error {{invalid input size for constraint 'x'}}
-  __asm__ volatile("foo1 %0" : "=x" (val256)); // expected-error {{invalid output size for constraint '=x'}}
-#endif
 }
Index: clang/lib/Basic/Targets/X86.cpp
===================================================================
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -1779,14 +1779,17 @@
     LLVM_FALLTHROUGH;
   case 'v':
   case 'x':
-    if (SSELevel >= AVX512F)
-      // 512-bit zmm registers can be used if target supports AVX512F.
-      return Size <= 512U;
-    else if (SSELevel >= AVX)
-      // 256-bit ymm registers can be used if target supports AVX.
-      return Size <= 256U;
-    return Size <= 128U;
-
+    return Size <= 512U;
+// FIXME: We should check the size based on the SSE level, but that requires
+// supporting the target attribute. The X86 backend seems to tolerate large
+// sizes and will produce an error message.
+//    if (SSELevel >= AVX512F)
+//      // 512-bit zmm registers can be used if target supports AVX512F.
+//      return Size <= 512U;
+//    else if (SSELevel >= AVX)
+//      // 256-bit ymm registers can be used if target supports AVX.
+//      return Size <= 256U;
+//    return Size <= 128U;
   }
 
   return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68255.222526.patch
Type: text/x-patch
Size: 3363 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191001/8880eb15/attachment.bin>