[PATCH] D144927: [GVNHoist] don't hoist callbr users into the callbr's block

Mon Mar 6 10:43:26 PST 2023

nikic added inline comments.

================
Comment at: llvm/test/Transforms/GVNHoist/hoist-call.ll:124
+
+; FIXME: it should be safe to hoist loads of variable not modified by callbr.
+ at y = global i32 0
----------------
nickdesaulniers wrote:
> I asked @george.burgess.iv if this fixme is correct; that we ought to be able to do such hoist. George mentioned that maybe AA was messing up. I wrote up a quick test case that disproves that: https://reviews.llvm.org/D145400 (not sure there's any value in landing that or not).  So pretty much not sure about this comment.
> 
> @efriedma thoughts?
What matters here is not the aliasing relationship with the callbr arguments (which have nothing to do with the callbr itself), but whether the callbr modrefs the location. I believe we don't have any special callbr handling for that, so it will be determined purely based on memory attributes. If you add something like `memory(argmem: readwrite)` that might work.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144927