[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Sema/JumpDiagnostics.cpp:658
 
+    if (auto *G = dyn_cast<GCCAsmStmt>(Jump)) {
+      for (AddrLabelExpr *L : G->labels()) {
----------------
nickdesaulniers wrote:
> rjmccall wrote:
> > I think it would be good to leave a comment here like this:
> > 
> > > We know the possible destinations of an asm goto, but we don't have the
> > > ability to add code along those control flow edges, so we have to diagnose
> > > jumps both in and out of scopes, just like we do with an indirect goto.
> Depending on your definition of "we" (clang vs. llvm), llvm does have the ability to add code along those edges.
> 
> See llvm/lib/CodeGen/CallBrPrepare.cpp.  I'll clarify in your comment that "clang does not split critical edges during code generation of llvm ... "
Okay, so, I don't really know much about this feature.  I was thinking that the branch might go directly into some other assembly block, which would not be splittable.  If the branch just goes to an arbitrary basic block in IR, then it would be straightforward for IRGen to just resolve the destination blocks for `asm goto` labels to some new block that does a normal `goto` to that label.  If we did that, we wouldn't need extra restrictions here at all and could just check this like any other direct branch.

We don't need to do that work right away, but the comment should probably reflect the full state of affairs — "but clang's IR generation does not currently know how to add code along these control flow edges, so we have to diagnose jumps both in and out of scopes, like we do with indirect goto.  If we ever add that ability to IRGen, this code could check these jumps just like ordinary `goto`s."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155342