[PATCH] D81909: [WebAssembly] Fix bug in FixBrTables and use branch analysis utils

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyFixBrTableDefaults.cpp:63
+           "Expected jump or fallthrough to br_table block");
+    MI.addOperand(MF, MachineOperand::CreateMBB(TBB));
   } else {
----------------
tlively wrote:
> aheejin wrote:
> > tlively wrote:
> > > tlively wrote:
> > > > aheejin wrote:
> > > > > Is it possible that both `TBB` and `FBB` are true and `FBB == MBB`?
> > > > Yes, here's what's possible.
> > > > 
> > > > '_' is nullptr, 'J' is the jump table block (aka MBB), 'D' is the default block
> > > > 
> > > > TBB | FBB | meaning
> > > > _ | _ | No default block, header falls through to jump table
> > > > J | _ | No default block, header jumps to jump table
> > > > D | _ | There is a default block, header falls through to jump table
> > > > D | J | There is a default block, header jumps to either default or jump table
> > > > 
> > > Actually, I'll add this table as a comment. It seems useful.
> > Yeah, I'm talking about the last case:
> > D | J | There is a default block, header jumps to either default or jump table
> > 
> > In this case, not `TBB`, but `FBB` should be the default target. I don't think this code does it.
> No, the condition that gets evaluated is whether the index is out of bounds, so the conditional jump (TBB) is to the default block and the unconditional jump (FBB) is to the jump table block (MBB). 
I thought it could be more robust to handle both ways, because we were depending on the particular way the branch is generated in SelectionDAG now. But probably it's an overkill. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81909