[PATCH] D136497: [Clang] support for outputs along indirect edges of asm goto

[Nick Desaulniers via Phabricator via cfe-commits]

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;
----------------
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);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136497