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

[Marina Yatsina via llvm-commits]

myatsina added inline comments.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:769
 
+  unsigned AdjustAVX512Mem(unsigned Size, OperandVector &Operands,
+                           unsigned UnsizedMemOpIndex);
----------------
Please add description above the function.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1179
     unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
-    InlineAsmIdentifierInfo &Info) {
+    InlineAsmIdentifierInfo &Info, bool IsBracketedMemOperand) {
   // If we found a decl other than a VarDecl, then assume it is a FuncDecl or
----------------
I think this name is a but misleading.
How about "KeepMemUnsized" or "AllowBetterSizeMatch"?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1209
+    if (IsBracketedMemOperand)
+      // handle cases where size qualifier is absent, upon an indirect symbol reference - i.e. "[var]"
+      // set Size to zero to allow matching mechansim to try and find a better
----------------
Please clang-format your patch.
handle --> Handle


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1211
+      // set Size to zero to allow matching mechansim to try and find a better
+      // size qualifier then our initial guess, based on available variants of
+      // the given instruction
----------------
then --> than


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2858
+  case 32:
+    break;
+  // currently - do not allow any other type of adjustment
----------------
why the break? why not put the inquiring logic here?
Is there some other code path that might reach the inquiring logic?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2895
       UnsizedMemOp = X86Op;
+      break;
+    }
----------------
Why the break?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2959
+    if (M == Match_Success)
+      // Allow a degenerate pan of SIMD related adjustments, to match (G|I)CC
+      // behavior on respective cases
----------------
pan?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2998
+      // MS compatibility - fix the rewrite according to the matched memory size
+      // We need to perform the fix online for MS inline assembly
+      for (AsmRewrite &AR : *InstInfo->AsmRewrites)
----------------
online --> only ?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:3000
+      for (AsmRewrite &AR : *InstInfo->AsmRewrites)
+        if ((AR.Loc.getPointer() == UnsizedMemOp->StartLoc.getPointer()) &&
+            (AR.Kind == AOK_SizeDirective))
----------------
a


Repository:
  rL LLVM

https://reviews.llvm.org/D26586