[PATCH] D43269: [MemorySSA] Be less aggressive with @llvm.lifetime.start

George Burgess IV via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 15 21:20:12 PST 2018


george.burgess.iv added reviewers: reames, hfinkel.
george.burgess.iv added a subscriber: gberry.
george.burgess.iv added a comment.

+hal and philip, as requested. +gberry in case they have any earlycse thoughts

> So, i don't know what we *want* the semantic to be.

Without much knowledge of the implications elsewhere, something like: "lifetime.start intrinsics represent a non-volatile, non-atomic clobber of the given range of memory for the purposes of analysis. Anything loaded from this range that has not been stored to since the lifetime.start is `undef`" doesn't sound insane to me.

(At least, I don't see why we need to give clobber mechanics to both lifetime.start and lifetime.end, given that AIUI they're meant to be balanced)

> Can you print the memoryssa dumps for this for me?



  define void @bar(i8* %foo) {
  entry:
    %z = bitcast i8* %foo to i32*
    %p = getelementptr inbounds i32, i32* %z, i64 1
  ; MemoryUse(liveOnEntry)
    %a = load i32, i32* %p
  ; 1 = MemoryDef(liveOnEntry)
    call void @llvm.lifetime.end.p0i8(i64 8, i8* %foo)
  ; 2 = MemoryDef(1)
    call void @llvm.lifetime.start.p0i8(i64 8, i8* %foo)
  ; 3 = MemoryDef(2)
    store i32 0, i32* %z
  ; 4 = MemoryDef(3)
    store i32 %a, i32* %p
    ret void
  }

It's all pretty straightforward IMO; the problematic interaction with EarlyCSE is that we don't report that `4` is clobbered by `2`, so AIUI it sees `4` as a redundant store.

> Essentially, lifetime.start/end, even for the same pointer, create things that are very two distinctly different memory objects, regardless of if they are at the same place in memory, and the compiler is free to treat them that way, at least, as these intrinsics go.

Agreed. So, at it's core, the DSE by EarlyCSE is effectively saying "I'm eliminating this store to object $X because it's a load of a value from object $Y that happens to reside at the same memory location."

(I realize I'm saying EarlyCSE a lot in a MSSA patch. The reason I've fixed it here is that I believe that, at the very least, MSSA users should have to opt into a 'be aggressive with lifetime intrinsics' footgun. Until there's complaining, ...)


Repository:
  rL LLVM

https://reviews.llvm.org/D43269





More information about the llvm-commits mailing list