<div dir="ltr"><div>> <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Yeah this isn't a helpful printout because i assume what happens is getClobberingMemoryAccess(4) returns LiveOnEntry</span></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Yup!</span></div><div><br></div><div>> <span style="font-size:12.8px">I will say really want to avoid going down the path we did with memdep here, where it would report fake clobbers and such.</span><br><br class="m_1600020718060687998m_4326582224351772236m_-4858826663189902226m_-6743994894692478490m_7029031442087839850m_274110258022686640m_8924769544951495787gmail-Apple-interchange-newline"></div><div>Yeah, I don't immediately see a good way to fold lifetimes in MSSA without treating turning lifetime.start/lifetime.end into some sort of clobber. If that's not a path we want to go down, I'm happy to stick with "MSSA cares about memory locations, not abstract objects," and drop this patch. I'll give the people I tagged time to respond though. :)</div><div><br></div><div>----</div><div><br></div><div>Also, sorry if I'm being dense, but if the goal is for MSSA to not care about lifetimes, and lifetimes aren't clobbers, why are we representing lifetime intrinsics in MSSA at all (alternatively: why does instructionClobbersQuery ever return true for lifetime_start)?<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 15, 2018 at 9:56 PM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_1600020718060687998m_4326582224351772236m_-4858826663189902226m_-6743994894692478490m_7029031442087839850m_274110258022686640m_8924769544951495787h5">On Thu, Feb 15, 2018 at 9:20 PM, George Burgess IV via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">george.burgess.iv added reviewers: reames, hfinkel.<br>
george.burgess.iv added a subscriber: gberry.<br>
george.burgess.iv added a comment.<br>
<br>
+hal and philip, as requested. +gberry in case they have any earlycse thoughts<br>
<span><br>
> So, i don't know what we *want* the semantic to be.<br>
<br>
</span>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.<br>
<br>
(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)<br>
<span><br>
> Can you print the memoryssa dumps for this for me?<br>
<br>
<br>
<br>
</span>  define void @bar(i8* %foo) {<br>
  entry:<br>
    %z = bitcast i8* %foo to i32*<br>
    %p = getelementptr inbounds i32, i32* %z, i64 1<br>
  ; MemoryUse(liveOnEntry)<br>
    %a = load i32, i32* %p<br>
  ; 1 = MemoryDef(liveOnEntry)<br>
    call void @llvm.lifetime.end.p0i8(i64 8, i8* %foo)<br>
  ; 2 = MemoryDef(1)<br>
    call void @llvm.lifetime.start.p0i8(i64 8, i8* %foo)<br>
  ; 3 = MemoryDef(2)<br>
    store i32 0, i32* %z<br>
  ; 4 = MemoryDef(3)<br>
    store i32 %a, i32* %p<br>
    ret void<br>
  }<br>
<br>
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.<br></blockquote><div><br></div></div></div><div>Yeah this isn't a helpful printout because i assume what happens is getClobberingMemoryAccess(4) returns LiveOnEntry or something.</div><div><br></div><div>Here, 4 *isn't* clobbered by 2.</div><div><br></div><div>I"m going ot reorg your message</div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">(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, ...)<br></blockquote><div><br></div></span><div>This is almost unrelated (IE it's not "aggressive" handling)</div><div>It's really a question about whether MemorySSA is talking about the memory locations, or the abstract memory objects that llvm has built using the lifetime system (which only really exist in the lifetime system).</div><div><br></div><div>If it's about the former, it's correct as-is.</div><div>If it's about the latter, yeah, then we should represent and push it.</div><div><br></div><div>The problem is we mix these all over the place, instead of being consistent.</div><div><br></div><div>I will say really want to avoid going down the path we did with memdep here, where it would report fake clobbers and such.</div><div><br></div><div>If we want these "memory objects" to be a first class citizen in LLVM, we should make them such, have the lifetime system return tokens, and use the tokens in loads/stores so you can tell what memory object they are talking about.</div><div><br></div><div>Right now it just doesn't exist outside of the lifetime system, and we are just taking the existing mess we have now, and trying to paper over it by pretending or not pretending, in various places, that lifetime intrinsics actually affect memory, depending on how we feel at a given time.</div><div><br></div><div><span><blockquote class="gmail_quote" style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_1600020718060687998m_4326582224351772236m_-4858826663189902226m_-6743994894692478490m_7029031442087839850m_274110258022686640m_8924769544951495787m_-7780080349704266232gmail-"><br class="m_1600020718060687998m_4326582224351772236m_-4858826663189902226m_-6743994894692478490m_7029031442087839850m_274110258022686640m_8924769544951495787m_-7780080349704266232gmail-Apple-interchange-newline"><br>> 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.<br><br></span>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."<br></blockquote><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><br></div></span><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">If memorySSA is about memory locations, it's earlycse being wrong by not also paying attention to lifetime regions.</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">In that situation, i don't think we should hack around it, i think we should fix it.</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><br></div></div></div></div></div>
</blockquote></div><br></div></div>