[PATCH] D69868: Allow "callbr" to return non-void values

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:1116
+    if (auto *cbr = dyn_cast<CallBrInst>(getBasicBlock()->getTerminator()))
+      if (cbr->getDefaultDest() != bb)
+        for (unsigned i = 0, e = cbr->getNumIndirectDests(); i != e; ++i)
----------------
nickdesaulniers wrote:
> void wrote:
> > rnk wrote:
> > > Is it possible for a BB to be both an indirect successor and a fallthrough successor? I suppose that could be the case with the Linux macro that gets the current PC.
> > > 
> > > In any case, it's probably safe to remove this condition, and then we don't have to worry.
> > It is possible. I have a testcase for it in this patch. :-)
> But you don't have a test case for what @rnk is asking about. (Unsure if it would be necessary).
> 
> I think @rnk is getting at this case from C:
> 
> ```
> void foo(void) {
>   asm goto ("#NICK":: "r"(&&hello) :: hello);
> hello:
>   return;
> }
> ```
> Clang will emit this as:
> ```
> define dso_local void @foo() #0 {
> entry:
>   callbr void asm sideeffect "#NICK", "r,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %hello), i8* blockaddress(@foo, %hello)) #1
>           to label %asm.fallthrough [label %hello], !srcloc !2
> 
> asm.fallthrough:                                  ; preds = %entry
>   br label %hello
> 
> hello:                                            ; preds = %asm.fallthrough, %entry
>   ret void
> }
> ```
> ie. the `blockaddress` is passed twice, once as the address of a label (GNU C extension), once as the indirect destination of the `asm goto`.
> 
> So to @rnk 's question:
> > Is it possible for a BB to be both an indirect successor and a fallthrough successor?
> It is valid LLVM IR to have a BB be both; Clang today (or with https://reviews.llvm.org/D69876) will not emit such formation (but could).
> 
> ie. imagine the above:
> 
> >  callbr void asm sideeffect "#NICK", "r,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %hello), i8* blockaddress(@foo, %hello)) #1
> >           to label %asm.fallthrough [label %hello], !srcloc !2
> 
> to instead be:
> 
> >  callbr void asm sideeffect "#NICK", "r,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %hello), i8* blockaddress(@foo, %asm.fallthrough)) #1
> >           to label %asm.fallthrough [label %asm.fallthrough], !srcloc !2
> 
> I still don't understand @rnk 's point about
> > In any case, it's probably safe to remove this condition, and then we don't have to worry.
> though.
I meant that I had a testcase that required the conditional he suggested I remove. Sorry for the confusion.

You're right that we can generate the clang code where the fallthrough is the same as the indirect. I didn't mean to imply that it wasn't the case. I can add a testcase. I asked a question on the "cfe-dev" mailing list to determine how I could identify the "fallthrough" block from the CFG. So far no one has responded. Until I can determine that, I won't be able to handle that properly with this conditional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868