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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

In D144927#4160250 <https://reviews.llvm.org/D144927#4160250>, @efriedma wrote:

> If you're going to enable hoisting past a callbr, please add a testcase to ensure we don't hoist a load past a callbr which modifies the memory in question.

Will do. Curiously, `GVNHoist::safeToHoistLdSt` returns `false` even for cases of loads not modified by callbr, so HoistGVN doesn't optimize those cases. FWICT, the call to `GVNHoist::firstInBB` is checking the callbr against itself. It looks like MemorySSA is perhaps claiming that for this input:

  @x = global i32 0
  @y = global i32 0
  define i32 @foo4(i1 %z) {
  entry:
    callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x)
            to label %a [label %b]
  
  a:                                                ; preds = %entry
    %0 = load i32, ptr @y, align 4
    ret i32 %0
  
  b:                                                ; preds = %entry
    %1 = load i32, ptr @y, align 4
    ret i32 %1
  }

that `callbr` is a "defining access" for the access of `@y` (IIUC)? That seems incorrect...but I'm unfamiliar with MemorySSA.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144927