[PATCH] D133914: [InlineAsm][bugfix] Correct function addressing in inline asm

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm added inline comments.


================
Comment at: llvm/lib/IR/InlineAsm.cpp:65
+  AsmStrs.clear();
+  if (AsmStr.empty())
+    return;
----------------
The StringRef has bug about do split for empty string. So I handle the empty case.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:32301-32306
+  StringRef InstrStr = getInstrStrFromOpNo(AsmStrs, OpNo);
+
+  // Do not use "startswith" here, because label may stay ahead.
+  // For example: ".L__MSASMLABEL_.${:uid}__l:call dword ptr ${0:P}"
+  if (InstrStr.contains("call"))
+    return true;
----------------
xiangzhangllvm wrote:
> xiangzhangllvm wrote:
> > pengfei wrote:
> > > The code seems fragile. How about using regex:
> > > ```
> > > std::string Call = formatv("call.*\\${?{0}", OpNo);
> > > for (auto &AsmStr : AsmStrs)
> > >   if (Regex(Call).match(AsmStr))
> > >     return true;
> > > return false;
> > > ```
> > good idea! Let me have a try, thanks a lot!
> Enhance in getInstrStrFromOpNo.
> Enhance in getInstrStrFromOpNo.




================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:32301-32306
+  StringRef InstrStr = getInstrStrFromOpNo(AsmStrs, OpNo);
+
+  // Do not use "startswith" here, because label may stay ahead.
+  // For example: ".L__MSASMLABEL_.${:uid}__l:call dword ptr ${0:P}"
+  if (InstrStr.contains("call"))
+    return true;
----------------
xiangzhangllvm wrote:
> xiangzhangllvm wrote:
> > xiangzhangllvm wrote:
> > > pengfei wrote:
> > > > The code seems fragile. How about using regex:
> > > > ```
> > > > std::string Call = formatv("call.*\\${?{0}", OpNo);
> > > > for (auto &AsmStr : AsmStrs)
> > > >   if (Regex(Call).match(AsmStr))
> > > >     return true;
> > > > return false;
> > > > ```
> > > good idea! Let me have a try, thanks a lot!
> > Enhance in getInstrStrFromOpNo.
> > Enhance in getInstrStrFromOpNo.
> 
> 
The regex can't handle the "$1" from "$12", and not clear for readability,  So I choose to enhance in getInstrStrFromOpNo (remove the label affect too).




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

https://reviews.llvm.org/D133914