<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 6, 2017 at 4:36 PM, Friedman, Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On 3/6/2017 11:06 AM, Wei Mi wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
After looking at the benchmark motivating the test, I find it is much harder for codegenprepare to catch all the shrinking opportunities compared with instcombine, because of two things:<br>
<br>
* loadpre may merge the original load in a shrinking pattern with other load in a very early position, and there maybe other stores in between. The safety check for the codegenprepare now simply scan mayWriteToMemory insns between load and store in the candidate shrinking pattern, and it will fail because of those stores in between.<br>
</blockquote>
<br></span>
It would be easy to make the check a lot more powerful using MemorySSA; I guess you don't have access to that in the passes where you're doing the transform?</blockquote><div><br></div><div>Yes, if MemorySSA + alias query can be used, it should solve the problem. I didn't find existing usage of MemorySSA in instcombine or CGP so I am not sure whether it can be used. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
* we may see the following pattern. a.f1 is a bitfield. Ideally, we should do the shrinking for all the three stores. But codegenprepare works in top-down order. After the first store is shrinked, it is hard for the second and the third to know %bf.set is the same as the value loaded from %0. If we do this in instcombine, it is much easier because before load/store elimination, %bf.set will be loaded from %0 respecitively in if.then9 and in if.end41.<br>
</blockquote>
<br></span>
I'm not following how it helps to do this in instcombine? Normally, EarlyCSE runs first. (A complete C testcase might be useful here.)</blockquote><div><br></div><div>It is not earlycse. It is also loadpre to cause the load to be replaced by %bf.set. </div><div><br></div><div><div style="font-size:12.8px">Before global value numbering:</div><div style="font-size:12.8px"><br class="gmail-Apple-interchange-newline">entry:</div><div style="font-size:12.8px"> %0 = bitcast %class.B* %this to i64*</div><div style="font-size:12.8px"> %bf.load = load i64, i64* %0, align 8</div><div style="font-size:12.8px"><div> %dec = add i64 %bf.load, 65535</div><div> %bf.value = and i64 %dec, 65535</div><div> %bf.clear5 = and i64 %bf.load, -65536</div><div> %bf.set = or i64 %bf.value, %bf.clear5</div><div> store i64 %bf.set, i64* %0, align 8</div></div><div style="font-size:12.8px"> ...</div><div style="font-size:12.8px">if.then9:</div><div style="font-size:12.8px"> <span style="font-size:12.8px">%bf.load1 = load i64, i64* %0, align 8 // %bf.load1 is helpful for the patch to recognize the load-and-or-store pattern.</span></div><div style="font-size:12.8px"><div> %inc79 = add i64 %bf.load1, 281474976710656 </div><div> %bf.shl = and i64 %inc79, 71776119061217280</div><div> %bf.clear18 = and i64 %bf.load1, -71776119061217281</div><div> %bf.set19 = or i64 %bf.shl, %bf.clear18</div><div> store i64 %bf.set19, i64* %0, align 8</div></div><div style="font-size:12.8px"> ...</div><div style="font-size:12.8px">if.end41:</div><div style="font-size:12.8px"> <span style="font-size:12.8px">%bf.load2 = load i64, i64* %0, align 8 </span><span style="font-size:12.8px">// %bf.load2 is helpful for the patch to recognize the load-and-or-store pattern.</span></div><div style="font-size:12.8px"><div> %inc4578 = add i64 %bf.load2, 65536 </div><div> %bf.shl48 = and i64 %inc4578, 4294901760</div><div> %bf.clear49 = and i64 %bf.load2, -4294901761</div><div> %bf.set50 = or i64 %bf.shl48, %bf.clear49</div><div> store i64 %bf.set50, i64* %0, align 8</div></div></div><div> </div><div>After global value numbering:</div><div><br></div><div><div style="font-size:12.8px">entry:</div><div style="font-size:12.8px"> %0 = bitcast %class.B* %this to i64*</div><div style="font-size:12.8px"> %bf.load = load i64, i64* %0, align 8</div><div style="font-size:12.8px"><div> %dec = add i64 %bf.load, 65535</div><div> %bf.value = and i64 %dec, 65535</div><div> %bf.clear5 = and i64 %bf.load, -65536</div><div> %bf.set = or i64 %bf.value, %bf.clear5</div><div> store i64 %bf.set, i64* %0, align 8</div></div><div style="font-size:12.8px"> ...</div><div style="font-size:12.8px">if.then9:</div><div style="font-size:12.8px"><div> %inc79 = add i64 %bf.set, 281474976710656 // we hope to know %bf.set is the same as load i64, i64* %0, align 8.</div><div> %bf.shl = and i64 %inc79, 71776119061217280</div><div> %bf.clear18 = and i64 %bf.set, -71776119061217281</div><div> %bf.set19 = or i64 %bf.shl, %bf.clear18</div><div> store i64 %bf.set19, i64* %0, align 8</div></div><div style="font-size:12.8px"> ...</div><div style="font-size:12.8px">if.end41:</div><div style="font-size:12.8px"><div> %inc4578 = add i64 %bf.set, 65536 // we hope to know %bf.set is the same as load i64, i64* %0, align 8.</div><div> %bf.shl48 = and i64 %inc4578, 4294901760</div><div> %bf.clear49 = and i64 %bf.set, -4294901761</div><div> %bf.set50 = or i64 %bf.shl48, %bf.clear49</div><div> store i64 %bf.set50, i64* %0, align 8</div><div><br></div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-HOEnZb"><font color="#888888"><br>
<br>
-Eli<br>
<br>
-- <br>
Employee of Qualcomm Innovation Center, Inc.<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project<br>
<br>
</font></span></blockquote></div><br></div></div>