<div dir="ltr">fixed a typo</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Mar 28, 2014 at 3:46 PM, Jingyue Wu <span dir="ltr"><<a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</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"><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="">On Fri, Mar 28, 2014 at 3:05 PM, Eli Bendersky <span dir="ltr"><<a href="mailto:eliben@google.com" target="_blank">eliben@google.com</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><br><div class="gmail_quote"><div>On Fri, Mar 28, 2014 at 2:25 PM, Jingyue Wu <span dir="ltr"><<a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a>></span> wrote:<br>



<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Hi Matt, <div><br></div><div>That makes sense, and here's a more constructive patch. <span><font color="#888888"><div>



<br></div></font></span></div></div></blockquote><div><br></div></div><div><div>+          if (StrippedPtrTy->getAddressSpace() == GEP.getAddressSpace())</div><div>+            return Res;</div><div>+          // Insert Res, and create an addrspacecast.</div>



<div>+          return new AddrSpaceCastInst(Builder->Insert(Res), GEP.getType());</div></div><div><br></div><div>Can you provide a bit more details in the comment? The surrounding code has examples like:</div><div><br>



</div><div>// -> GEP ... stuff</div><div><br></div><div>Maybe something like this to clarify the sequence it turns to.</div></div></div></div></blockquote><div><br></div></div><div>Done</div><div class=""><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div><br></div><div><div>+@array = internal addrspace(3) global [256 x float] zeroinitializer, align 4</div>

<div>+</div><div>+define float* @keep_necessary_addrspacecast(i64 %i) {</div><div>+entry:</div><div>+; CHECK-LABEL: @keep_necessary_addrspacecast</div><div>+; CHECK: addrspacecast float addrspace(3)* %{{[0-9]+}} to float*</div>



<div>+  %0 = getelementptr [256 x float]* addrspacecast ([256 x float] addrspace(3)* @array to [256 x float]*), i64 0, i64 %i</div><div>+  ret float* %0</div><div>+}</div></div><div><br></div><div><br></div><div>Please create test cases that cover both paths in the patch (array / not-array).</div>

</div></div></div></blockquote><div><br></div></div><div>Done</div><div class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote">
<span><font color="#888888">

<div><br></div><div>Eli</div></font></span><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<div dir="ltr"><div>

<span><font color="#888888"><div></div><div>Jingyue</div></font></span></div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Mar 28, 2014 at 12:07 PM, Matt Arsenault <span dir="ltr"><<a href="mailto:Matthew.Arsenault@amd.com" target="_blank">Matthew.Arsenault@amd.com</a>></span> wrote:<br>




<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
  
    
  
  <div text="#000000" bgcolor="#FFFFFF"><div><div>
    <div>On 03/28/2014 12:01 PM, Jingyue Wu
      wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">
        <div>Fixing PR19270. This issue is blocking a waiting patch of
          mine that implements the optimization we discussed in <a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-March/071440.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-March/071440.html</a>. </div>





        <div><br>
        </div>
        visitGetElementPtr in InstructionCombining.cpp gets rid of
        unnecessary pointer casts in gep (cast X). However, this
        optimization may change the address space of the result pointer
        type, and cause type mismatch.
        <div>
          <br>
        </div>
        <div>e.g.,</div>
        <div>getelementptr [256 x float]* addrspacecast ([256 x float]
          addrspace(3)* @array to [256 x float]*), i64 0, i64 %i</div>
        <div>returns a float*, but the optimized instruction</div>
        <div>getelementptr [256 x float] addrspace(3)* @array, i64 0,
          i64 %i<br>
        </div>
        <div>returns a float addrspace(3)*</div>
        <div>
          <div>
            <div><br>
              <div>The attached patch disables this optimization when
                the address space of the source is different from that
                of the destination.</div>
            </div>
          </div>
        </div>
        <div>
          <br>
        </div>
        <div>Jingyue</div>
      </div>
    </blockquote>
    <br></div></div>
    If we're doing what I suggested and trying to do operations in the
    original address space, then instead of disabling this, why don't
    you make this instead just move the addrspacecast to after the GEP?
    Do the GEP in addrspace(3), and then addrspacecast that.<span><font color="#888888"><br>
    <br>
    -Matt<br>
  </font></span></div>

</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div></div><br></div></div>
</blockquote></div><br></div>