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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp:470
+                     !MI->getOperand(OpNo).isBlockAddress() &&
+                     !MI->getOperand(OpNo).isMBB()) {
+            Error = true;
----------------
void wrote:
> nickdesaulniers wrote:
> > 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.
> The compiler should do this already, because `MI` is `const` and `getOperand` has no side-effects. I think that's who no one has bothered to do it before.
No doubt, but it's annoying to continually type out the full expression, hence the request to DRY up the code. Please make that change.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp:470
+                     !MI->getOperand(OpNo).isBlockAddress() &&
+                     !MI->getOperand(OpNo).isMBB()) {
+            Error = true;
----------------
Technically, here `!MI->getOperand(OpNo).isBlockAddress() && !MI->getOperand(OpNo).isMBB()` is tautalogically false (due to the earlier checks), so you'd only need to check for `l`. Maybe a comment like `// %l should have been handled by the above conditions already.`, too would be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71849