<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 2, 2016 at 9:51 AM, Bryant Wong <span dir="ltr"><<a href="mailto:3.14472+reviews.llvm.org@gmail.com" target="_blank">3.14472+reviews.llvm.org@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">>> Here's an example where upwards motion is necessary for the transformation<br>
>> to<br>
>> occur:<br>
>><br>
>> %S = type { i8*, i8, i32 }<br>
>><br>
>> define void @noaliasaddrproducer(%S* %src, %S* noalias %dst,<br>
>> %S* noalias %dstidptr) {<br>
>> ; MemoryUse(liveOnEntry)<br>
>> %1 = load %S, %S* %src ; combine this into a memcpy...<br>
>> ; 1 = MemoryDef(liveOnEntry)<br>
>> store %S zeroinitializer, %S* %src<br>
>> %y = bitcast %S* %dstidptr to i8*<br>
>> ; 2 = MemoryDef(1)<br>
>> call void @llvm.memset.p0i8.i64(i8* %y, i8 0, i64 54, i32 0, i1 false)<br>
>> ; 3 = MemoryDef(2)<br>
>> store %S %1, %S* %dstidptr ; ...with this.<br>
>> ret void<br>
>> }<br>
>><br>
>> The store to %src prevents %1 from being moved down to its corresponding<br>
>> store,<br>
><br>
> so instead, the corresponding store is moved up to %1.<br>
><br>
><br>
</span><span class="">> I agree this is how it is currently done.<br>
> I disagree this is the only way to achieve this, and that trying to do it<br>
> this way necessarily makes sense with MemorySSA.<br>
><br>
> The final result is:<br>
</span><span class="">> define void @noaliasaddrproducer(%S* %src, %S* noalias %dst, %S* noalias<br>
> %dstidptr) {<br>
</span><span class="">> %1 = bitcast %S* %src to i8*<br>
</span><span class="">> %y = bitcast %S* %dstidptr to i8*<br>
</span><span class="">> %2 = getelementptr i8, i8* %y, i64 16<br>
> call void @llvm.memset.p0i8.i64(i8* %2, i8 0, i64 38, i32 8, i1 false)<br>
> call void @llvm.memcpy.p0i8.p0i8.i64(i8* %y, i8* %1, i64 16, i32 8, i1<br>
> false)<br>
> call void @llvm.memset.p0i8.i64(i8* %1, i8 0, i64 16, i32 8, i1 false)<br>
> ret void<br>
> }<br>
><br>
> In order (and this is not even the only way to do this)<br>
><br>
> The store of the zero initializer is processed, you walk the use def chain,<br>
> discover it is all zeroes. It is replaced with a memset. Since you are<br>
> replacing a single store with a single memset, it works fine.<br>
><br>
> You go to the next store.<br>
> You walk the use use-def chain from the memorydef at 3, stopping at each<br>
> piece, and seeing what info it provides, by examining the instruction. For<br>
> each definingAccess, you can examine the uses to see if they provide loads<br>
> that similarly fill in part of the range.<br>
> You discover that 0, 16 has value %1 coming from a load, and 16, 54 is<br>
> coming from zero.<br>
><br>
> You create the memcpy and memset to make this happen, and replace the load<br>
> and store.<br>
><br>
> None of this *requires* inserting and then later worrying about things.<br>
><br>
> What am i missing?<br>
<br>
</span>The point of that example was that AccessList splicing is needed.</blockquote><div><br></div><div>There are certainly times AccessList splicing would be needed to perform an operation, i'd agree with this :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Nothing to do<br>
with insertions. Sorry for the confusion, and it was a bad example anyway<br>
because the intermediate memset above the final store is must-alias with the<br>
final store's pointer. Can we try again?<br>
<span class=""><br>
%S = type { i8*, i8, i32 }<br>
</span> declare %S* @compute(%S*)<br>
<br>
define void @f(%S* noalias %a, %S* %b) {<br>
; MemoryUse(liveOnEntry)<br>
%1 = load %S, %S* %a<br>
; 1 = MemoryDef(liveOnEntry)<br>
store %S zeroinitializer, %S* %a ; prevents %1 from being moved down<br>
; 2 = MemoryDef(1)<br>
%ptr = call %S* @compute(%S* %b) ; must be moved up with 3<br>
; 3 = MemoryDef(2)<br>
store %S %1, %S* %ptr<br>
ret void<br>
}<br>
<br>
(For brevity, I'm going to refer to each store instruction by their MemoryDef<br>
ID.)<br>
<br>
Similar to my previous crappy example, 1 blocks the downward motion of<br>
%1 = load, so 3 has to move up above 1 before %1 + 3 can be combined into a<br>
memcpy. Also, 2 must move upwards with 3 because 3 depends on it.<br></blockquote><div><br></div><div>See, this is again the part i don't get.</div><div>You are doing all this to remove these instructions. Why do you need to do it, update memory ssa, do some more work, and then remove the instructions.</div><div>Why can't you simply analyze, determine what you need to do, and produce the end result all at once?<br>This requires infinitely less updating work.</div><div>Let's assume you don't want to go that route (i will eventually go that route, FWIW, and replace most of this logic :P)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
So 3 must be spliced above 1, and splicing consists of: RAU 3 W 2; RAU 1 W 3;<br>
setDefiningAccess of 3 to 1's defining access. The last part isn't possible<br>
because sDA isn't public for non-MemoryUses. </blockquote><div><br></div><div>Yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Also, if for some reason we needed<br>
to splice a MemoryDef ahead of its defining MemoryDef, RAUW wouldn't work.<br>
</blockquote></div><br></div><div class="gmail_extra">Also correct, but that would generate invalid code, of course, as defs would not dominate uses :)</div><div class="gmail_extra"><br>The next question i have is: "Do you really believe you can perform all these updates yourself, correctly, no matter what the CFG looks like, etc".</div><div class="gmail_extra">My guess, honestly, is "no"</div><div class="gmail_extra"><br></div><div class="gmail_extra">I can trivially change the CFG above such that you would have to use IDFCalculator to place memoryphi nodes yourself.</div><div class="gmail_extra"> </div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div></div>