[PATCH] D121785: The [3/3] Fix mangle problem when variable used in inline asm （Support ARR[BaseReg+IndexReg+..] in PIC model）

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:489-491
+      // This case mostly happen in inline asm, e.g. Arr[BaseReg + IndexReg]
+      // can not intruduce additional register in inline asm in PIC model.
+      if (IsPIC && (!SymName.empty() || AttachToOperandIdx))
----------------
xiangzhangllvm wrote:
> skan wrote:
> > Why do you need to check `AttachToOperandIdx` and `SymName` at same time?
> Because in back end, (when the source code is *.ll), the GV will be present like $0 , we should use AttachToOperandIdx to make sure there is an operand.
Please add comments about this. 

If I understood it right, the check `!SymName.empty()` was for front end, shouldn't we add a C test for it?


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:843
+        // model (e.g. i386-darwin).
+        // Here we remove "- BaseSym" in inlineasm if it exists.
+        // TODO(RefineMe): Here is an architecture defect for both FE parser
----------------
inline asm


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121785/new/

https://reviews.llvm.org/D121785