[PATCH] D130316: [SelectionDAG] make INLINEASM_BR use MachineBasicBlocks instead of BlockAddresses

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 22 10:01:01 PDT 2022


nickdesaulniers added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/callbr-asm-label.ll:10
 ; CHECK: // %bb.1:
-; CHECK: .Ltmp0:
+; CHECK: .Ltmp1:
 ; CHECK: .LBB0_2: // %indirect
----------------
barannikov88 wrote:
> I'm a bit out of context, so the question might be silly, sorry.
> Shouldn't this temporary label no longer be emitted? It used to be referenced, but no longer is.
> 
It's not a silly question; it's a great question.

I've been trying to track that down, as I don't know the answer myself.

IIRC, we did previously have an issue with labels disappearing out from under us. @void solved that in either

- commit 83d690a14980 ("Don't rely on 'l'(ell) modifiers to indicate a label reference")
- commit 1b104388752f ("[MC] Don't recreate a label if it's already used")

I've already found that the latter is no longer necessary, hence https://reviews.llvm.org/D130323, but that's not what was retaining these additional symbols.

(or maybe it was a third commit, maybe 79176a2542d03107b90613c84f18ccba41ad8fa8?)

So I would like to find the answer to this question, and perhaps remove the duplicate labels.  But it would be a minor cosmetic fix; I'm not going to spend too much time on it since this patch is just a chain in a longer series that we're ultimately trying to fix.  Fixing duplicate labels is nice to have, but slightly orthogonal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130316



More information about the llvm-commits mailing list