<div dir="ltr">Very odd. The only change was to alias analysis of memory operations to constants so it should all be very local. <div><br></div><div><div>I've reverted the change for now and will look at it over the holidays.<br></div><div><br></div><div>-Nirav</div><div><div><br></div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 22, 2017 at 1:45 PM, Michael Kruse <span dir="ltr"><<a href="mailto:llvm-commits@meinersbur.de" target="_blank">llvm-commits@meinersbur.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
this commit caused the miscompilation of three test-suite programs:<br>
<br>
LinearDependence-dbl.execution<wbr>_time<br>
tramp3d-v4.execution_time<br>
pointer_member.execution_time<br>
<br>
See:<br>
<a href="http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-unprofitable/builds/10187" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/perf-x86_64-penryn-O3-<wbr>polly-unprofitable/builds/1018<wbr>7</a><br>
<br>
(Obsequi.execution_time was fixed by me in r321371)<br>
<br>
Could you have a look why this NFCI commit caused this? Thank you.<br>
<br>
Michael<br>
<br>
<br>
<br>
<br>
2017-12-22 17:59 GMT+01:00 Nirav Dave via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>:<br>
<div class="m_-8893846820880777508HOEnZb"><div class="m_-8893846820880777508h5">> Author: niravd<br>
> Date: Fri Dec 22 08:59:09 2017<br>
> New Revision: 321364<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=321364&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=321364&view=rev</a><br>
> Log:<br>
> [DAG] Integrate findBaseOffset address analyses to BaseIndexOffset. NFCI.<br>
><br>
> BaseIndexOffset supercedes findBaseOffset analysis save only Constant<br>
> Pool addresses. Migrate analysis to BaseIndexOffset.<br>
><br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/Select<wbr>ionDAG/DAGCombiner.cpp<br>
> llvm/trunk/lib/CodeGen/Select<wbr>ionDAG/SelectionDAGAddressAnal<wbr>ysis.cpp<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/DAGCombiner.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=321364&r1=321363&r2=321364&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp?<wbr>rev=321364&r1=321363&r2=<wbr>321364&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/DAGCombiner.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/DAGCombiner.cpp Fri Dec 22 08:59:09 2017<br>
> @@ -17405,43 +17405,6 @@ SDValue DAGCombiner::buildSqrtEstimate<wbr>(S<br>
> return buildSqrtEstimateImpl(Op, Flags, false);<br>
> }<br>
><br>
> -/// Return true if base is a frame index, which is known not to alias with<br>
> -/// anything but itself. Provides base object and offset as results.<br>
> -static bool findBaseOffset(SDValue Ptr, SDValue &Base, int64_t &Offset,<br>
> - const GlobalValue *&GV, const void *&CV) {<br>
> - // Assume it is a primitive operation.<br>
> - Base = Ptr; Offset = 0; GV = nullptr; CV = nullptr;<br>
> -<br>
> - // If it's an adding a simple constant then integrate the offset.<br>
> - if (Base.getOpcode() == ISD::ADD) {<br>
> - if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Base.<wbr>getOperand(1))) {<br>
> - Base = Base.getOperand(0);<br>
> - Offset += C->getSExtValue();<br>
> - }<br>
> - }<br>
> -<br>
> - // Return the underlying GlobalValue, and update the Offset. Return false<br>
> - // for GlobalAddressSDNode since the same GlobalAddress may be represented<br>
> - // by multiple nodes with different offsets.<br>
> - if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(<wbr>Base)) {<br>
> - GV = G->getGlobal();<br>
> - Offset += G->getOffset();<br>
> - return false;<br>
> - }<br>
> -<br>
> - // Return the underlying Constant value, and update the Offset. Return false<br>
> - // for ConstantSDNodes since the same constant pool entry may be represented<br>
> - // by multiple nodes with different offsets.<br>
> - if (ConstantPoolSDNode *C = dyn_cast<ConstantPoolSDNode>(B<wbr>ase)) {<br>
> - CV = C->isMachineConstantPoolEntry(<wbr>) ? (const void *)C->getMachineCPVal()<br>
> - : (const void *)C->getConstVal();<br>
> - Offset += C->getOffset();<br>
> - return false;<br>
> - }<br>
> - // If it's any of the following then it can't alias with anything but itself.<br>
> - return isa<FrameIndexSDNode>(Base);<br>
> -}<br>
> -<br>
> /// Return true if there is any possibility that the two addresses overlap.<br>
> bool DAGCombiner::isAlias(LSBaseSDN<wbr>ode *Op0, LSBaseSDNode *Op1) const {<br>
> // If they are the same then they must be aliases.<br>
> @@ -17483,39 +17446,16 @@ bool DAGCombiner::isAlias(LSBaseSDN<wbr>ode *<br>
> return false;<br>
> }<br>
><br>
> - // FIXME: findBaseOffset and ConstantValue/GlobalValue/Fram<wbr>eIndex analysis<br>
> - // modified to use BaseIndexOffset.<br>
> -<br>
> - // Gather base node and offset information.<br>
> - SDValue Base0, Base1;<br>
> - int64_t Offset0, Offset1;<br>
> - const GlobalValue *GV0, *GV1;<br>
> - const void *CV0, *CV1;<br>
> - bool IsFrameIndex0 = findBaseOffset(Op0->getBasePtr<wbr>(),<br>
> - Base0, Offset0, GV0, CV0);<br>
> - bool IsFrameIndex1 = findBaseOffset(Op1->getBasePtr<wbr>(),<br>
> - Base1, Offset1, GV1, CV1);<br>
> -<br>
> - // If they have the same base address, then check to see if they overlap.<br>
> - if (Base0 == Base1 || (GV0 && (GV0 == GV1)) || (CV0 && (CV0 == CV1)))<br>
> - return !((Offset0 + NumBytes0) <= Offset1 ||<br>
> - (Offset1 + NumBytes1) <= Offset0);<br>
> -<br>
> - // It is possible for different frame indices to alias each other, mostly<br>
> - // when tail call optimization reuses return address slots for arguments.<br>
> - // To catch this case, look up the actual index of frame indices to compute<br>
> - // the real alias relationship.<br>
> - if (IsFrameIndex0 && IsFrameIndex1) {<br>
> - MachineFrameInfo &MFI = DAG.getMachineFunction().getFr<wbr>ameInfo();<br>
> - Offset0 += MFI.getObjectOffset(cast<Frame<wbr>IndexSDNode>(Base0)->getIndex(<wbr>));<br>
> - Offset1 += MFI.getObjectOffset(cast<Frame<wbr>IndexSDNode>(Base1)->getIndex(<wbr>));<br>
> - return !((Offset0 + NumBytes0) <= Offset1 ||<br>
> - (Offset1 + NumBytes1) <= Offset0);<br>
> - }<br>
> -<br>
> - // Otherwise, if we know what the bases are, and they aren't identical, then<br>
> - // we know they cannot alias.<br>
> - if ((IsFrameIndex0 || CV0 || GV0) && (IsFrameIndex1 || CV1 || GV1))<br>
> + bool IsFI0 = isa<FrameIndexSDNode>(BasePtr0<wbr>.getBase());<br>
> + bool IsFI1 = isa<FrameIndexSDNode>(BasePtr1<wbr>.getBase());<br>
> + bool IsGV0 = isa<GlobalAddressSDNode>(BaseP<wbr>tr0.getBase());<br>
> + bool IsGV1 = isa<GlobalAddressSDNode>(BaseP<wbr>tr1.getBase());<br>
> + bool IsCV0 = isa<ConstantPoolSDNode>(BasePt<wbr>r0.getBase());<br>
> + bool IsCV1 = isa<ConstantPoolSDNode>(BasePt<wbr>r1.getBase());<br>
> +<br>
> + // Matched types are already handled above. So if this is true they<br>
> + // cannot alias.<br>
> + if ((IsFI0 || IsGV0 || IsCV0) && (IsFI1 || IsGV1 || IsCV1))<br>
> return false;<br>
><br>
> // If we know required SrcValue1 and SrcValue2 have relatively large alignment<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/SelectionDAGAddressAnaly<wbr>sis.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp?rev=321364&r1=321363&r2=321364&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/SelectionDAGAddre<wbr>ssAnalysis.cpp?rev=321364&r1=<wbr>321363&r2=321364&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/SelectionDAGAddressAnaly<wbr>sis.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/SelectionDAGAddressAnaly<wbr>sis.cpp Fri Dec 22 08:59:09 2017<br>
> @@ -37,6 +37,23 @@ bool BaseIndexOffset::equalBaseInde<wbr>x(Bas<br>
> return true;<br>
> }<br>
><br>
> + // Match Constants<br>
> + if (auto *A = dyn_cast<ConstantPoolSDNode>(B<wbr>ase))<br>
> + if (auto *B = dyn_cast<ConstantPoolSDNode>(O<wbr>ther.Base)) {<br>
> + bool IsMatch =<br>
> + A->isMachineConstantPoolEntry(<wbr>) == B->isMachineConstantPoolEntry(<wbr>);<br>
> + if (IsMatch) {<br>
> + if (A->isMachineConstantPoolEntry<wbr>())<br>
> + IsMatch = A->getMachineCPVal() == B->getMachineCPVal();<br>
> + else<br>
> + IsMatch = A->getConstVal() == B->getConstVal();<br>
> + }<br>
> + if (IsMatch) {<br>
> + Off += B->getOffset() - A->getOffset();<br>
> + return true;<br>
> + }<br>
> + }<br>
> +<br>
> const MachineFrameInfo &MFI = DAG.getMachineFunction().getFr<wbr>ameInfo();<br>
><br>
> // Match non-equal FrameIndexes - If both frame indices are fixed<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div></div></div></div>