[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:132
+  // Place new instruction sequence after GEP.
+  Builder.SetInsertPoint(GEP);
+  Value *Index = GEP->getOperand(2);
----------------
This line causes the bug seen in bind. In that case, the GEP has been hoisted, but the load has not. In general the GEP could be in a different basic block, or even in the same basic block with an instruction that may not return (intrinsic, real function call, well-defined language-level exception, etc). You can insert the reltable.shift where the GEP is, and that probably makes sense given it serves (part of) the same purpose, but you *must* insert the actual reltable.intrinsic where the original load is, unless you've gone to great lengths to prove it's safe not to (which seems best left to the usual culprits like LICM).

IR test cases: https://godbolt.org/z/YMdaMrobE (bind is characterised by the first of the two functions)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355