<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 14, 2015 at 12:59 PM, JF Bastien <span dir="ltr"><<a href="mailto:jfb@google.com" target="_blank">jfb@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"><div class="gmail_quote"><span class="">On Mon, Sep 14, 2015 at 11:25 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.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"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Mon, Sep 14, 2015 at 11:13 AM, JF Bastien <span dir="ltr"><<a href="mailto:jfb@google.com" target="_blank">jfb@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"><div class="gmail_extra"><div class="gmail_quote"><span><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 class="gmail_extra"><div class="gmail_quote"><span><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">GetElementPointers must have the first argument's type compared<br>
for structural equivalence. Previously the code erroneously compared the<br>
pointer's type, but this code was dead because all pointer types (of the<br>
same address space) are the same. The pointee must be compared instead<br>
(using the type stored in the GEP, not from the pointer type which will<br>
be erased anyway).<br></blockquote></span><div><br>Thanks so much for being aware of this/using APIs that'll be forwards compatible with the eventual removal of typed pointers.<br><br>Just in case: do you care about the GEP pointer operands address space (I assume not) or vector types (eg: a gep over a vector of pointers)? I assume not, but just checking - both those aspects of the pointer operand are still in the pointer operand's type, not in the source element type.</div></div></div></div></blockquote><div><br></div></span><div>We do care about address spaces in general: we can't merge two functions if they refer to different address spaces. I'm not sure that's what you're asking for though: in the end we (roughly) only want to merge functions that would codegen to the same code. I think the same applies to your vector of pointers question?</div></div></div></div></blockquote><div><br></div></div></div><div>My point was - do you care if these two GEPs refer to different pointers from address spaces? What if the GEPs are differently vectored (if vector GEPs can exist in this codepath, etc)?<br></div></div></div></div></blockquote><div><br></div></span><div>Yes, we care if the address spaces are different. That's checked at the start of the function.</div><div><br></div><div>By vectored you mean the indexing? I think this is handled properly, but it would be great if you could confirm.</div></div></div></div></blockquote><div><br></div><div>I really don't know enough about this pass or code to know if this is even reproducible. Can you get vector geps in this codepath? <br><br>getelementptr (i32, [5 x i32*] @x, ... )<br><br><br></div><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 class=""><div><br></div><div><br></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 class="gmail_extra"><div class="gmail_quote"><div>If you do care about the (in)equality of address space or vector-ness in these two GEPs, you'll need to compare those - those would've previously been caught by just comparing the two GEPs pointer operand types, but won't be any more with this change.</div></div></div></div></blockquote><div><br></div><div><br></div></span></div>For reference, the current code is:</div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace">int FunctionComparator::cmpGEPs(const GEPOperator *GEPL,</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> const GEPOperator *GEPR) {</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"><br></font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> unsigned int ASL = GEPL->getPointerAddressSpace();</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> unsigned int ASR = GEPR->getPointerAddressSpace();</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"><br></font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> if (int Res = cmpNumbers(ASL, ASR))</font></div></div></blockquote></div></blockquote><div><br>Ah, thanks - sorry, hadn't looked at the actual code.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> return Res;</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"><br></font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> // When we have target data, we can reduce the GEP down to the value in bytes</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> // added to the address.</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> const DataLayout &DL = FnL->getParent()->getDataLayout();</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> unsigned BitWidth = DL.getPointerSizeInBits(ASL);</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> APInt OffsetL(BitWidth, 0), OffsetR(BitWidth, 0);</font></div></div><span class=""><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> if (GEPL->accumulateConstantOffset(DL, OffsetL) &&</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> GEPR->accumulateConstantOffset(DL, OffsetR))</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> return cmpAPInts(OffsetL, OffsetR);</font></div></div></span><span class=""><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> if (int Res = cmpTypes(GEPL->getSourceElementType(),</font></div></div></span><span class=""><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> GEPR->getSourceElementType()))</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> return Res;</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"><br></font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> if (int Res = cmpNumbers(GEPL->getNumOperands(), GEPR->getNumOperands()))</font></div></div></span><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> return Res;</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"><br></font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> for (unsigned i = 0, e = GEPL->getNumOperands(); i != e; ++i) {</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> if (int Res = cmpValues(GEPL->getOperand(i), GEPR->getOperand(i)))</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> return Res;</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> }</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"><br></font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace"> return 0;</font></div></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace">}</font></div></div></blockquote><div class="gmail_extra"><div><br></div></div></div>
</blockquote></div><br></div></div>