[PATCH] D64888: Use the MachineBasicBlock symbol for a callbr target

[Bill Wendling via Phabricator via llvm-commits]

void added a comment.

In D64888#1596336 <https://reviews.llvm.org/D64888#1596336>, @hans wrote:

> In D64888#1593116 <https://reviews.llvm.org/D64888#1593116>, @xbolva00 wrote:
>
> > It would be nice to merge this fix into 9.0 branch after some time in trunk. What do you think?
> >
> > @hans
>
>
> Sounds like this was reverted on trunk. Please let me know if there are follow-ups that might be candidates for merging.


I have a patch in my local branch that should fix some of the issues. However, I can't seem to support the situation Nick pointed out. If you have something like:

  define void @foo() {
  entry:
    callbr void asm sideeffect ".word ${0:l}", "X"(i8* blockaddress(@bar, %next))
            to label %if.end10 [label %l_yes]
  
  if.end10:
    br label %l_yes
  
  l_yes:
    ret void
  }
  
  define i32 @bar() {
  entry:
    br label %next
  
  next:
    ret i32 42
  }

This will fail, because `@bar` hasn't been created yet, so it can't get the address of `next`. Or, rather, it *can*, but the assembly parser won't realize that it's a label and will create a new label for it. So it goes from this:

  foo:
      .word .Ltmp0
    ...
  bar:
  .Ltmp0:

to this:

  foo:
      .word .Ltmp00  ; <- Notice the '0' suffix, which is applied to a label to uniquify it.
    ...
  bar:
  .Ltmp0:

when parsing the inline assembly. I *think* we need to convince `MCContext` that `.Ltmp0` is a label that it doesn't need to uniquify—i.e., register it as a symbol in `MCContext` instead of treating it as a temporary label (there's a difference, and I'm not sure why).

The question is whether this is a case we need to care about at this point in time.

Thoughts?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64888