[PATCH] D115688: [SelectionDAG] treat X constrained labels as i for asm

[James Y Knight via Phabricator via llvm-commits]

jyknight added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1683
   }
+  if (const auto *BB = dyn_cast<BasicBlock>(V))
+    return DAG.getBasicBlock(FuncInfo.MBBMap[BB]);
----------------
How does BasicBlock get here, vs BlockAddress?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5083
 
-    if (Op.getNode() && Op.getOpcode() == ISD::TargetBlockAddress)
+    if (isa<BasicBlock>(v) || isa<BlockAddress>(v)) {
+      OpInfo.ConstraintCode = "i";
----------------
I'm still confused as to why this deals in both BlockAddress and BasicBlock types -- why aren't the values all BlockAddresses?


================
Comment at: llvm/test/CodeGen/AArch64/inlineasm-X-constraint.ll:130
 ; CHECK-LABEL: f7
 ; CHECK: bl
 define void @f7() {
----------------
Can now "CHECK: bl bb"?


================
Comment at: llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll:84
 ; PPC64LE-NEXT:    #NO_APP
-; PPC64LE-NEXT:  # %bb.1: # %return
+; PPC64LE-NEXT: .LBB3_1: # %return
 ; PPC64LE-NEXT:    extsw r3, r3
----------------
The asm fallthrough grew a label it didn't need before? And...that affected the need to save/restore lr or something?

Hmm...wait...it sure looks like the expected code in this test was demonstrating a bug...! It wasn't saving/restoring lr (mflr/mtlr) around the callbr, but the callbr said it clobbers lr! That doesn't seem right -- despite the test asserting it...

OK...but why did this change fix that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115688