[PATCH] D26586: [X86][AVX512][InlineASM][MS][llvm] (I|G)CC Memory adjustments compatibility

[coby via llvm-commits]

coby added inline comments.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2848
+  // Check for the existence of an AVX512 platform
+  if (!getSTI().getFeatureBits()[X86::FeatureAVX512])
+    return 0;
----------------
delena wrote:
> Put "assert" instead of "if".
Cannot assert at this point. This function is called regardless of the platform. 


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2885
   for (const auto &Op : Operands) {
     X86Operand *X86Op = static_cast<X86Operand *>(Op.get());
+    if (UnsizedMemOp && !UnsizedMemOpNext)
----------------
delena wrote:
> IA allows only one memory operand per instruction.
> if (UnsizedMemOp) {
>       UnsizedMemOpNext = X86Op;
>       break;
> }
> UnsizedMemOp = X86Op;
> 
Previous revision addressed this issue exactly, and was rejected. I found it redundant (and wrong) to continue looking past the first interception of an unsized mem op.
Having said that - both approaches are congregated to the same behavior.
If Marina won't furthermore object - I'm keen to revert to previous approach.


Repository:
  rL LLVM

https://reviews.llvm.org/D26586