[PATCH] D71849: Don't rely on 'l'(ell) modifiers to indicate a label reference

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 6 09:47:16 PST 2020


nickdesaulniers added a comment.

Thanks for the fix. Two minor fixes requested, then should be good to land.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp:470
+                     !MI->getOperand(OpNo).isBlockAddress() &&
+                     !MI->getOperand(OpNo).isMBB()) {
+            Error = true;
----------------
Would you mind hoisting the expression `MI->getOperand(OpNo)` into a local variable defined above near the definition of `OpNo`? It would DRY up this code.


================
Comment at: llvm/test/CodeGen/X86/callbr-asm.ll:135
+; Test 4 - asm-goto referenced with the 'l' (ell) modifier and not.
+define dso_local void @test4() {
+; CHECK-LABEL: test4:
----------------
do we require `dso_local` now? (`opt -verify <file>` says we don't). Would you mind dropping it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71849





More information about the llvm-commits mailing list