[PATCH] D69868: Allow "callbr" to return non-void values
Bill Wendling via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 17 15:24:52 PST 2019
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
More information about the cfe-commits
mailing list