[PATCH] D25005: [x86][inline-asm][llvm] accept 'v' constraint

[Elena Demikhovsky via llvm-commits]

delena added a comment.

In my opinion, you did not write enough tests. You should cover AVX512F more with zmm, ymm, xmm, multiple instructions.
You also should check error messages, when user puts wrong registers.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32036
@@ -32030,3 +32035,3 @@
   case 'Y':
     if (((type->getPrimitiveSizeInBits() == 128) && Subtarget.hasSSE1()) ||
         ((type->getPrimitiveSizeInBits() == 256) && Subtarget.hasFp256()))
----------------
does 'v' work without avx512?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32386
@@ -32376,1 +32385,3 @@
+        if (VConstraint && Subtarget.hasAVX512() && Subtarget.hasVLX())
+          return std::make_pair(0U, &X86::FR64XRegClass);
         return std::make_pair(0U, &X86::FR64RegClass);
----------------
FR64 and FR64X will not work for i64 and i32 . Please add a test that checks this.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32406
@@ -32392,1 +32405,3 @@
       case MVT::v4f64:
+        if (VConstraint && Subtarget.hasAVX512() && Subtarget.hasVLX())
+          return std::make_pair(0U, &X86::VR256XRegClass);
----------------
you can remove Subtarget.hasAVX512(). VLX includes AV512

================
Comment at: test/CodeGen/X86/inline-asm-avx512f-v-constraint.ll:2
@@ +1,3 @@
+; RUN: llc < %s -march x86-64 -mtriple x86_64-unknown-linux-gnu -mattr +avx512vl | FileCheck %s
+; RUN: llc < %s -march x86-64 -mtriple x86_64-unknown-linux-gnu -mattr +avx512f -mattr -avx512vl | FileCheck %s
+
----------------
-mattr -avx512vl is redundant, you can remove the second run.


Repository:
  rL LLVM

https://reviews.llvm.org/D25005