<div dir="ltr">Ah, I suspect you're right. Every non-intrinsic that's not a LoadInst, but loads memory, also modifies it, so would be<div>caught by the earlier check. I think you were right that just changing the isa to mayReadFromMemory should be sufficient.</div><div>For a test, you can use any in-tree intrinsic that reads memory e.g. `llvm.load.relative`.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 11, 2018 at 2:22 PM,  <span dir="ltr"><<a href="mailto:escha@apple.com" target="_blank">escha@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><br><div><span class=""><br><blockquote type="cite"><div>On Jan 11, 2018, at 11:03 AM, Keno Fischer <<a href="mailto:keno@juliacomputing.com" target="_blank">keno@juliacomputing.com</a>> wrote:</div><br class="m_1986404414453306531Apple-interchange-newline"><div><div dir="ltr">I haven't looked at this code in a while, but I think you'll have to<div><br></div><div>1) Add an `else if (Inst->mayReadFromMemory()) { return false; }` after the check for LoadInst in isSafeToMove as a</div><div>    correctness fix.</div></div></div></blockquote><div><br></div></span><div>Is this necessary? I’m fairly sure this should already be covered here:</div><div><br></div><div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;background-color:rgb(255,255,255)"><span style="color:#ba2da2">if</span> (<span style="color:#ba2da2">auto</span> CS = CallSite(Inst)) {</div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,132,0);background-color:rgb(255,255,255)"><span style="color:#000000">    </span>// Convergent operations cannot be made control-dependent on additional</div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,132,0);background-color:rgb(255,255,255)"><span style="color:#000000">    </span>// values.</div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;background-color:rgb(255,255,255)">    <span style="color:#ba2da2">if</span> (CS.hasFnAttr(Attribute::<wbr>Convergent))</div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(186,45,162);background-color:rgb(255,255,255)"><span style="color:#000000">      </span>return<span style="color:#000000"> </span>false<span style="color:#000000">;</span></div><div style="margin:0px;font-stretch:normal;line-height:normal;background-color:rgb(255,255,255);min-height:14px"><br></div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;background-color:rgb(255,255,255)">    <span style="color:#ba2da2">for</span> (Instruction *S : Stores)</div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;background-color:rgb(255,255,255)">      <span style="color:#ba2da2">if</span> (AA.getModRefInfo(S, CS) & MRI_Mod)</div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(186,45,162);background-color:rgb(255,255,255)"><span style="color:#000000">        </span>return<span style="color:#000000"> </span>false<span style="color:#000000">;</span></div><div style="margin:0px;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;background-color:rgb(255,255,255)">  }</div></div><span class=""><br><blockquote type="cite"><div><div dir="ltr"><div>2) For memory reading instructions you care about add a similar check to the one that's there for LoadInst to recover</div><div>    the optimization (if applicable)</div><div>3) Change the isa<LoadInst> to mayReadFromMemory in IsAcceptableTarget</div><div>4) Add a test for your regression ;)</div></div></div></blockquote><br></span></div><div>i’m not sure about the test because this only comes up (for us, at least) with out-of-tree intrinsics that access memory (ex: texture reads).</div><div><br></div><div>—escha</div></div></blockquote></div><br></div>