nickdesaulniers added inline comments.
================
Comment at: clang/lib/CodeGen/CGStmt.cpp:2868-2873
+ // If we happen to share the same indirect and default dest, don't re-add
+ // stores. That was done for the default destination in the above call to
+ // EmitAsmStores.
+ llvm::BasicBlock *Succ = CBR->getIndirectDest(i);
+ if (Succ == CBR->getDefaultDest())
+ continue;
----------------
nickdesaulniers wrote:
> void wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > I'm not sure that I need to handle this case (of a callbr with the same
> > > > indirect destination as the default destination). That might result
> > > > from optimizations, but I don't think clang can or will generate such
> > > > IR. Maybe I should turn this into an: `assert(CBR->getIndirectDest(i)
> > > > != CBR->getDefaultDest(i) && "We already emitted asm stores in the
> > > > default dest");`?
> > > Yeah, it's impossible. While a CallBrInst can have a same indirect
> > > destination as the default destination as a result of optimizations,
> > > above we always create an `"asm.fallthrough"` block that is distinct from
> > > any of the named labels from the `asm goto`.
> > It could only happen with something like this:
> >
> > ```
> > asm goto (... :::: indirect);
> >
> > indirect:;
> > ```
> >
> > In that simplified case, it looks like clang emits this:
> >
> > ```
> > define dso_local void @foo() #0 {
> > callbr void asm sideeffect "", "i,~{dirflag},~{fpsr},~{flags}"(i8*
> > blockaddress(@foo, %2)) #1
> > to label %1 [label %2], !srcloc !6
> >
> > 1: ; preds = %0
> > br label %2
> >
> > 2: ; preds = %1, %0
> > ret void
> > }
> > ```
> >
> > So it's probably safe to make this an assert.
> Oh, it's not just the default and indirects sharing, but what if the same
> indirect is listed twice? I should add a test for that...i.e.
>
> ```
> asm goto (... :::: indirect,indirect);
> ```
Oh, right, we don't allow that.
```
error: duplicate use of asm operand name "indirect"
note: asm operand name "indirect" first referenced here
```
I'm going to leave these asserts out and mark this thread done, but please
reopen this thread with a new comment if you feel strongly that I should add it
back (`assert(CBR->getIndirectDest(i) != CBR->getDefaultDest(i) && "We already
emitted asm stores in the default dest");`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136497/new/
https://reviews.llvm.org/D136497
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
