[PATCH] D25005: [x86][inline-asm][llvm] accept 'v' constraint
Elena Demikhovsky via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 29 01:15:21 PDT 2016
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
More information about the llvm-commits
mailing list