[PATCH] D157124: [CodeGen][AArch64] Don't split jump table basic blocks

[Mingming Liu via Phabricator via llvm-commits]

mingmingl added a comment.

thanks! Mostly LG with one comment from compile-time perspective



================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:8391-8398
+  auto isInJumpTable = [&](const MachineJumpTableEntry &JTE) {
+    return llvm::find(JTE.MBBs, &MBB) != JTE.MBBs.end();
+  };
+  auto isJumpTableTarget = [&](const MachineBasicBlock &MBB) {
+    const MachineJumpTableInfo *MJTI = MBB.getParent()->getJumpTableInfo();
+    return MJTI != nullptr &&
+           llvm::any_of(MJTI->getJumpTables(), isInJumpTable);
----------------
Mostly from compile-time perspective,  could this be pre-computed per MachineFunction in machine-function-splitter pass for AArch64?

Something like:

```
// precompute 'JumpTableTargets' for each machine function 
const auto& MJTI = F->getJumpTableInfo();

SmallSet<MachineBasicBlock*> JumpTableTargetsSet;

for (auto JT : MJTI->getJumpTables()) {
  for (auto* MBB : JT.MBBs) { 
    JumpTableTargetsSet.insert(MBB);
  }
}
```

// As long as 'JumpTableTargetsSet' remains valid (i.e., not invalidated by any code transformations, like branch folding, etc)
// For each 'AArch64InstrInfo::isMBBSafeToSplitToCold' method call
```
auto isJumpTableTarget = [&](const MachineBasicBlock& MBB) {
  return JumpTableTargets.count(MBB);
}
```


================
Comment at: llvm/test/CodeGen/AArch64/machine-function-splitter.ll:32-34
+cold1:                                            ; preds = %0
+  %3 = tail call i32 @bam()
+  ret i32 %3
----------------
For my understanding, is `cold1` split out to `nosplit_jumptable.cold` before this patch?  


================
Comment at: llvm/test/CodeGen/AArch64/machine-function-splitter.ll:63
+!15 = !{!"function_section_prefix", !"hot"}
+!16 = !{!"branch_weights", i32 7000, i32 0}
+!17 = !{!"branch_weights", i32 0, i32 4000, i32 4000, i32 0, i32 1000}
----------------
nit: if this is not used, maybe remove this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157124