[llvm] [SandboxIR] Preserve the order of switch cases after revert. (PR #115577)

Jorge Gorbe Moya via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 11 15:05:40 PST 2024


================
@@ -170,7 +170,19 @@ void CatchSwitchAddHandler::revert(Tracker &Tracker) {
   LLVMCSI->removeHandler(LLVMCSI->handler_begin() + HandlerIdx);
 }
 
-void SwitchRemoveCase::revert(Tracker &Tracker) { Switch->addCase(Val, Dest); }
+void SwitchRemoveCase::revert(Tracker &Tracker) {
+  // removeCase swaps the last case with the deleted one. To revert it, we use
+  // addCase (which adds the new case to the end), and swap the newly-added
+  // value and successor operands to the positions for the original case index.
+  Switch->addCase(Val, Dest);
+  auto ValUseA = Switch->getOperandUse(2 + Index * 2);
+  auto SucUseA = Switch->getOperandUse(2 + Index * 2 + 1);
+  unsigned NumOps = Switch->getNumOperands();
+  auto ValUseB = Switch->getOperandUse(NumOps - 2);
+  auto SucUseB = Switch->getOperandUse(NumOps - 2 + 1);
+  ValUseA.swap(ValUseB);
----------------
slackito wrote:

> I think a comment that explains how this works would be really helpful.

That's what this comment a couple of lines above was supposed to be about:
```
  // removeCase swaps the last case with the deleted one. To revert it, we use
  // addCase (which adds the new case to the end), and swap the newly-added
  // value and successor operands to the positions for the original case index.
```
I can try to rephrase/expand it if it wasn't clear enough.

> Hmm but should we rely on this behavior ? it seems too implementation specific, so if someone changes the implementation in LLVM IR, our tests will break.

Yeah, I was assuming the implementation wasn't very likely to change and that we would be fine updating this logic if it ever happens. If we don't feel comfortable with that option I can go with the alternative you suggested (recording all cases, then adding them again on revert) instead. Which one do you prefer?

https://github.com/llvm/llvm-project/pull/115577


More information about the llvm-commits mailing list