[llvm] r322003 - [DAG] Teach BaseIndexOffset to correctly handle with indexed operations
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 17 09:50:02 PST 2018
Merged to 6.0 in r322693.
On Mon, Jan 8, 2018 at 5:21 PM, Nirav Dave via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: niravd
> Date: Mon Jan 8 08:21:35 2018
> New Revision: 322003
>
> URL: http://llvm.org/viewvc/llvm-project?rev=322003&view=rev
> Log:
> [DAG] Teach BaseIndexOffset to correctly handle with indexed operations
>
> BaseIndexOffset address analysis incorrectly ignores offsets folded
> into indexed memory operations causing potential errors in alias
> analysis of pre-indexed operations.
>
> Reviewers: efriedma, RKSimon, hfinkel, jyknight
>
> Subscribers: hiraditya, javed.absar, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D41701
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h
> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h?rev=322003&r1=322002&r2=322003&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h Mon Jan 8 08:21:35 2018
> @@ -56,7 +56,7 @@ public:
> int64_t &Off);
>
> /// Parses tree in Ptr for base, index, offset addresses.
> - static BaseIndexOffset match(SDValue Ptr, const SelectionDAG &DAG);
> + static BaseIndexOffset match(LSBaseSDNode *N, const SelectionDAG &DAG);
> };
>
> } // end namespace llvm
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=322003&r1=322002&r2=322003&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Jan 8 08:21:35 2018
> @@ -5225,7 +5225,7 @@ SDValue DAGCombiner::MatchLoadCombine(SD
> return SDValue();
>
> // Loads must share the same base address
> - BaseIndexOffset Ptr = BaseIndexOffset::match(L->getBasePtr(), DAG);
> + BaseIndexOffset Ptr = BaseIndexOffset::match(L, DAG);
> int64_t ByteOffsetFromBase = 0;
> if (!Base)
> Base = Ptr;
> @@ -12944,7 +12944,7 @@ void DAGCombiner::getStoreMergeCandidate
> StoreSDNode *St, SmallVectorImpl<MemOpLink> &StoreNodes) {
> // This holds the base pointer, index, and the offset in bytes from the base
> // pointer.
> - BaseIndexOffset BasePtr = BaseIndexOffset::match(St->getBasePtr(), DAG);
> + BaseIndexOffset BasePtr = BaseIndexOffset::match(St, DAG);
> EVT MemVT = St->getMemoryVT();
>
> SDValue Val = peekThroughBitcast(St->getValue());
> @@ -12965,7 +12965,7 @@ void DAGCombiner::getStoreMergeCandidate
> EVT LoadVT;
> if (IsLoadSrc) {
> auto *Ld = cast<LoadSDNode>(Val);
> - LBasePtr = BaseIndexOffset::match(Ld->getBasePtr(), DAG);
> + LBasePtr = BaseIndexOffset::match(Ld, DAG);
> LoadVT = Ld->getMemoryVT();
> // Load and store should be the same type.
> if (MemVT != LoadVT)
> @@ -12984,7 +12984,7 @@ void DAGCombiner::getStoreMergeCandidate
> return false;
> // The Load's Base Ptr must also match
> if (LoadSDNode *OtherLd = dyn_cast<LoadSDNode>(Val)) {
> - auto LPtr = BaseIndexOffset::match(OtherLd->getBasePtr(), DAG);
> + auto LPtr = BaseIndexOffset::match(OtherLd, DAG);
> if (LoadVT != OtherLd->getMemoryVT())
> return false;
> if (!(LBasePtr.equalBaseIndex(LPtr, DAG)))
> @@ -13008,7 +13008,7 @@ void DAGCombiner::getStoreMergeCandidate
> Val.getOpcode() != ISD::EXTRACT_SUBVECTOR)
> return false;
> }
> - Ptr = BaseIndexOffset::match(Other->getBasePtr(), DAG);
> + Ptr = BaseIndexOffset::match(Other, DAG);
> return (BasePtr.equalBaseIndex(Ptr, DAG, Offset));
> };
>
> @@ -13381,7 +13381,7 @@ bool DAGCombiner::MergeConsecutiveStores
> if (Ld->getMemoryVT() != MemVT)
> break;
>
> - BaseIndexOffset LdPtr = BaseIndexOffset::match(Ld->getBasePtr(), DAG);
> + BaseIndexOffset LdPtr = BaseIndexOffset::match(Ld, DAG);
> // If this is not the first ptr that we check.
> int64_t LdOffset = 0;
> if (LdBasePtr.getBase().getNode()) {
> @@ -17452,44 +17452,46 @@ bool DAGCombiner::isAlias(LSBaseSDNode *
> unsigned NumBytes1 = Op1->getMemoryVT().getStoreSize();
>
> // Check for BaseIndexOffset matching.
> - BaseIndexOffset BasePtr0 = BaseIndexOffset::match(Op0->getBasePtr(), DAG);
> - BaseIndexOffset BasePtr1 = BaseIndexOffset::match(Op1->getBasePtr(), DAG);
> + BaseIndexOffset BasePtr0 = BaseIndexOffset::match(Op0, DAG);
> + BaseIndexOffset BasePtr1 = BaseIndexOffset::match(Op1, DAG);
> int64_t PtrDiff;
> - if (BasePtr0.equalBaseIndex(BasePtr1, DAG, PtrDiff))
> - return !((NumBytes0 <= PtrDiff) || (PtrDiff + NumBytes1 <= 0));
> -
> - // If both BasePtr0 and BasePtr1 are FrameIndexes, we will not be
> - // able to calculate their relative offset if at least one arises
> - // from an alloca. However, these allocas cannot overlap and we
> - // can infer there is no alias.
> - if (auto *A = dyn_cast<FrameIndexSDNode>(BasePtr0.getBase()))
> - if (auto *B = dyn_cast<FrameIndexSDNode>(BasePtr1.getBase())) {
> - MachineFrameInfo &MFI = DAG.getMachineFunction().getFrameInfo();
> - // If the base are the same frame index but the we couldn't find a
> - // constant offset, (indices are different) be conservative.
> - if (A != B && (!MFI.isFixedObjectIndex(A->getIndex()) ||
> - !MFI.isFixedObjectIndex(B->getIndex())))
> - return false;
> - }
> + if (BasePtr0.getBase().getNode() && BasePtr1.getBase().getNode()) {
> + if (BasePtr0.equalBaseIndex(BasePtr1, DAG, PtrDiff))
> + return !((NumBytes0 <= PtrDiff) || (PtrDiff + NumBytes1 <= 0));
> +
> + // If both BasePtr0 and BasePtr1 are FrameIndexes, we will not be
> + // able to calculate their relative offset if at least one arises
> + // from an alloca. However, these allocas cannot overlap and we
> + // can infer there is no alias.
> + if (auto *A = dyn_cast<FrameIndexSDNode>(BasePtr0.getBase()))
> + if (auto *B = dyn_cast<FrameIndexSDNode>(BasePtr1.getBase())) {
> + MachineFrameInfo &MFI = DAG.getMachineFunction().getFrameInfo();
> + // If the base are the same frame index but the we couldn't find a
> + // constant offset, (indices are different) be conservative.
> + if (A != B && (!MFI.isFixedObjectIndex(A->getIndex()) ||
> + !MFI.isFixedObjectIndex(B->getIndex())))
> + return false;
> + }
>
> - bool IsFI0 = isa<FrameIndexSDNode>(BasePtr0.getBase());
> - bool IsFI1 = isa<FrameIndexSDNode>(BasePtr1.getBase());
> - bool IsGV0 = isa<GlobalAddressSDNode>(BasePtr0.getBase());
> - bool IsGV1 = isa<GlobalAddressSDNode>(BasePtr1.getBase());
> - bool IsCV0 = isa<ConstantPoolSDNode>(BasePtr0.getBase());
> - bool IsCV1 = isa<ConstantPoolSDNode>(BasePtr1.getBase());
> -
> - // If of mismatched base types or checkable indices we can check
> - // they do not alias.
> - if ((BasePtr0.getIndex() == BasePtr1.getIndex() || (IsFI0 != IsFI1) ||
> - (IsGV0 != IsGV1) || (IsCV0 != IsCV1)) &&
> - (IsFI0 || IsGV0 || IsCV0) && (IsFI1 || IsGV1 || IsCV1))
> - return false;
> + bool IsFI0 = isa<FrameIndexSDNode>(BasePtr0.getBase());
> + bool IsFI1 = isa<FrameIndexSDNode>(BasePtr1.getBase());
> + bool IsGV0 = isa<GlobalAddressSDNode>(BasePtr0.getBase());
> + bool IsGV1 = isa<GlobalAddressSDNode>(BasePtr1.getBase());
> + bool IsCV0 = isa<ConstantPoolSDNode>(BasePtr0.getBase());
> + bool IsCV1 = isa<ConstantPoolSDNode>(BasePtr1.getBase());
> +
> + // If of mismatched base types or checkable indices we can check
> + // they do not alias.
> + if ((BasePtr0.getIndex() == BasePtr1.getIndex() || (IsFI0 != IsFI1) ||
> + (IsGV0 != IsGV1) || (IsCV0 != IsCV1)) &&
> + (IsFI0 || IsGV0 || IsCV0) && (IsFI1 || IsGV1 || IsCV1))
> + return false;
> + }
>
> - // If we know required SrcValue1 and SrcValue2 have relatively large alignment
> - // compared to the size and offset of the access, we may be able to prove they
> - // do not alias. This check is conservative for now to catch cases created by
> - // splitting vector types.
> + // If we know required SrcValue1 and SrcValue2 have relatively large
> + // alignment compared to the size and offset of the access, we may be able
> + // to prove they do not alias. This check is conservative for now to catch
> + // cases created by splitting vector types.
> int64_t SrcValOffset0 = Op0->getSrcValueOffset();
> int64_t SrcValOffset1 = Op1->getSrcValueOffset();
> unsigned OrigAlignment0 = Op0->getOriginalAlignment();
> @@ -17499,8 +17501,8 @@ bool DAGCombiner::isAlias(LSBaseSDNode *
> int64_t OffAlign0 = SrcValOffset0 % OrigAlignment0;
> int64_t OffAlign1 = SrcValOffset1 % OrigAlignment1;
>
> - // There is no overlap between these relatively aligned accesses of similar
> - // size. Return no alias.
> + // There is no overlap between these relatively aligned accesses of
> + // similar size. Return no alias.
> if ((OffAlign0 + NumBytes0) <= OffAlign1 ||
> (OffAlign1 + NumBytes1) <= OffAlign0)
> return false;
> @@ -17663,7 +17665,7 @@ bool DAGCombiner::findBetterNeighborChai
>
> // This holds the base pointer, index, and the offset in bytes from the base
> // pointer.
> - BaseIndexOffset BasePtr = BaseIndexOffset::match(St->getBasePtr(), DAG);
> + BaseIndexOffset BasePtr = BaseIndexOffset::match(St, DAG);
>
> // We must have a base and an offset.
> if (!BasePtr.getBase().getNode())
> @@ -17689,7 +17691,7 @@ bool DAGCombiner::findBetterNeighborChai
> break;
>
> // Find the base pointer and offset for this memory node.
> - BaseIndexOffset Ptr = BaseIndexOffset::match(Index->getBasePtr(), DAG);
> + BaseIndexOffset Ptr = BaseIndexOffset::match(Index, DAG);
>
> // Check that the base pointer is the same as the original one.
> if (!BasePtr.equalBaseIndex(Ptr, DAG))
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=322003&r1=322002&r2=322003&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Mon Jan 8 08:21:35 2018
> @@ -7947,11 +7947,8 @@ bool SelectionDAG::areNonVolatileConsecu
> if (VT.getSizeInBits() / 8 != Bytes)
> return false;
>
> - SDValue Loc = LD->getOperand(1);
> - SDValue BaseLoc = Base->getOperand(1);
> -
> - auto BaseLocDecomp = BaseIndexOffset::match(BaseLoc, *this);
> - auto LocDecomp = BaseIndexOffset::match(Loc, *this);
> + auto BaseLocDecomp = BaseIndexOffset::match(Base, *this);
> + auto LocDecomp = BaseIndexOffset::match(LD, *this);
>
> int64_t Offset = 0;
> if (BaseLocDecomp.equalBaseIndex(LocDecomp, *this, Offset))
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp?rev=322003&r1=322002&r2=322003&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp Mon Jan 8 08:21:35 2018
> @@ -21,6 +21,9 @@ using namespace llvm;
>
> bool BaseIndexOffset::equalBaseIndex(BaseIndexOffset &Other,
> const SelectionDAG &DAG, int64_t &Off) {
> + // Conservatively fail if we a match failed..
> + if (!Base.getNode() || !Other.Base.getNode())
> + return false;
> // Initial Offset difference.
> Off = Other.Offset - Offset;
>
> @@ -72,13 +75,29 @@ bool BaseIndexOffset::equalBaseIndex(Bas
> }
>
> /// Parses tree in Ptr for base, index, offset addresses.
> -BaseIndexOffset BaseIndexOffset::match(SDValue Ptr, const SelectionDAG &DAG) {
> +BaseIndexOffset BaseIndexOffset::match(LSBaseSDNode *N,
> + const SelectionDAG &DAG) {
> + SDValue Ptr = N->getBasePtr();
> +
> // (((B + I*M) + c)) + c ...
> SDValue Base = DAG.getTargetLoweringInfo().unwrapAddress(Ptr);
> SDValue Index = SDValue();
> int64_t Offset = 0;
> bool IsIndexSignExt = false;
>
> + // pre-inc/pre-dec ops are components of EA.
> + if (N->getAddressingMode() == ISD::PRE_INC) {
> + if (auto *C = dyn_cast<ConstantSDNode>(N->getOffset()))
> + Offset += C->getSExtValue();
> + else // If unknown, give up now.
> + return BaseIndexOffset(SDValue(), SDValue(), 0, false);
> + } else if (N->getAddressingMode() == ISD::PRE_DEC) {
> + if (auto *C = dyn_cast<ConstantSDNode>(N->getOffset()))
> + Offset -= C->getSExtValue();
> + else // If unknown, give up now.
> + return BaseIndexOffset(SDValue(), SDValue(), 0, false);
> + }
> +
> // Consume constant adds & ors with appropriate masking.
> while (Base->getOpcode() == ISD::ADD || Base->getOpcode() == ISD::OR) {
> if (auto *C = dyn_cast<ConstantSDNode>(Base->getOperand(1))) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list