From 55ccd16354d90a70e890f4a79b9bfd20186b3c5f Mon Sep 17 00:00:00 2001 From: Evgeniy Stepanov Date: Mon, 19 Aug 2019 21:08:04 +0000 Subject: [PATCH] Refactor isPointerOffset (NFC). Summary: Simplify the API using Optional<> and address comments in https://reviews.llvm.org/D66165 Reviewers: vitalybuka Subscribers: hiraditya, llvm-commits, ostannard, pcc Tags: #llvm Differential Revision: https://reviews.llvm.org/D66317 llvm-svn: 369300 --- llvm/include/llvm/Analysis/ValueTracking.h | 10 +++--- llvm/lib/Analysis/ValueTracking.cpp | 43 +++++++++++-------------- llvm/lib/Target/AArch64/AArch64StackTagging.cpp | 14 ++++---- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp | 14 ++++---- 4 files changed, 38 insertions(+), 43 deletions(-) diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h index bd37cbe..182cad2 100644 --- a/llvm/include/llvm/Analysis/ValueTracking.h +++ b/llvm/include/llvm/Analysis/ValueTracking.h @@ -661,11 +661,11 @@ class Value; const Instruction *ContextI, const DataLayout &DL); - /// Return true if Ptr1 is provably equal to Ptr2 plus a constant offset, and - /// return that constant offset. For example, Ptr1 might be &A[42], and Ptr2 - /// might be &A[40]. In this case offset would be -8. - bool isPointerOffset(Value *Ptr1, Value *Ptr2, int64_t &Offset, - const DataLayout &DL); + /// If Ptr1 is provably equal to Ptr2 plus a constant offset, return that + /// offset. For example, Ptr1 might be &A[42], and Ptr2 might be &A[40]. In + /// this case offset would be -8. + Optional isPointerOffset(const Value *Ptr1, const Value *Ptr2, + const DataLayout &DL); } // end namespace llvm #endif // LLVM_ANALYSIS_VALUETRACKING_H diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index 127c3dc..a8ee594 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -5702,9 +5702,8 @@ ConstantRange llvm::computeConstantRange(const Value *V, bool UseInstrInfo) { return CR; } -static int64_t getOffsetFromIndex(const GEPOperator *GEP, unsigned Idx, - bool &VariableIdxFound, - const DataLayout &DL) { +static Optional +getOffsetFromIndex(const GEPOperator *GEP, unsigned Idx, const DataLayout &DL) { // Skip over the first indices. gep_type_iterator GTI = gep_type_begin(GEP); for (unsigned i = 1; i != Idx; ++i, ++GTI) @@ -5715,7 +5714,7 @@ static int64_t getOffsetFromIndex(const GEPOperator *GEP, unsigned Idx, for (unsigned i = Idx, e = GEP->getNumOperands(); i != e; ++i, ++GTI) { ConstantInt *OpC = dyn_cast(GEP->getOperand(i)); if (!OpC) - return VariableIdxFound = true; + return None; if (OpC->isZero()) continue; // No offset. @@ -5734,32 +5733,30 @@ static int64_t getOffsetFromIndex(const GEPOperator *GEP, unsigned Idx, return Offset; } -bool llvm::isPointerOffset(Value *Ptr1, Value *Ptr2, int64_t &Offset, - const DataLayout &DL) { +Optional llvm::isPointerOffset(const Value *Ptr1, const Value *Ptr2, + const DataLayout &DL) { Ptr1 = Ptr1->stripPointerCasts(); Ptr2 = Ptr2->stripPointerCasts(); // Handle the trivial case first. if (Ptr1 == Ptr2) { - Offset = 0; - return true; + return 0; } - GEPOperator *GEP1 = dyn_cast(Ptr1); - GEPOperator *GEP2 = dyn_cast(Ptr2); - - bool VariableIdxFound = false; + const GEPOperator *GEP1 = dyn_cast(Ptr1); + const GEPOperator *GEP2 = dyn_cast(Ptr2); // If one pointer is a GEP and the other isn't, then see if the GEP is a // constant offset from the base, as in "P" and "gep P, 1". if (GEP1 && !GEP2 && GEP1->getOperand(0)->stripPointerCasts() == Ptr2) { - Offset = -getOffsetFromIndex(GEP1, 1, VariableIdxFound, DL); - return !VariableIdxFound; + auto Offset = getOffsetFromIndex(GEP1, 1, DL); + if (!Offset) + return None; + return -*Offset; } if (GEP2 && !GEP1 && GEP2->getOperand(0)->stripPointerCasts() == Ptr1) { - Offset = getOffsetFromIndex(GEP2, 1, VariableIdxFound, DL); - return !VariableIdxFound; + return getOffsetFromIndex(GEP2, 1, DL); } // Right now we handle the case when Ptr1/Ptr2 are both GEPs with an identical @@ -5768,7 +5765,7 @@ bool llvm::isPointerOffset(Value *Ptr1, Value *Ptr2, int64_t &Offset, // offset, which determines their offset from each other. At this point, we // handle no other case. if (!GEP1 || !GEP2 || GEP1->getOperand(0) != GEP2->getOperand(0)) - return false; + return None; // Skip any common indices and track the GEP types. unsigned Idx = 1; @@ -5776,11 +5773,9 @@ bool llvm::isPointerOffset(Value *Ptr1, Value *Ptr2, int64_t &Offset, if (GEP1->getOperand(Idx) != GEP2->getOperand(Idx)) break; - int64_t Offset1 = getOffsetFromIndex(GEP1, Idx, VariableIdxFound, DL); - int64_t Offset2 = getOffsetFromIndex(GEP2, Idx, VariableIdxFound, DL); - if (VariableIdxFound) - return false; - - Offset = Offset2 - Offset1; - return true; + auto Offset1 = getOffsetFromIndex(GEP1, Idx, DL); + auto Offset2 = getOffsetFromIndex(GEP2, Idx, DL); + if (!Offset1 || !Offset2) + return None; + return *Offset2 - *Offset1; } diff --git a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp index 44f7499..4dfe743 100644 --- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp +++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp @@ -358,12 +358,12 @@ Instruction *AArch64StackTagging::collectInitializers(Instruction *StartInst, break; // Check to see if this store is to a constant offset from the start ptr. - int64_t Offset; - if (!isPointerOffset(StartPtr, NextStore->getPointerOperand(), Offset, - *DL)) + Optional Offset = + isPointerOffset(StartPtr, NextStore->getPointerOperand(), *DL); + if (!Offset) break; - if (!IB.addStore(Offset, NextStore, DL)) + if (!IB.addStore(*Offset, NextStore, DL)) break; LastInst = NextStore; } else { @@ -376,11 +376,11 @@ Instruction *AArch64StackTagging::collectInitializers(Instruction *StartInst, break; // Check to see if this store is to a constant offset from the start ptr. - int64_t Offset; - if (!isPointerOffset(StartPtr, MSI->getDest(), Offset, *DL)) + Optional Offset = isPointerOffset(StartPtr, MSI->getDest(), *DL); + if (!Offset) break; - if (!IB.addMemSet(Offset, MSI)) + if (!IB.addMemSet(*Offset, MSI)) break; LastInst = MSI; } diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp index 79bf6e0..89520ec 100644 --- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp +++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp @@ -335,12 +335,12 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst, break; // Check to see if this store is to a constant offset from the start ptr. - int64_t Offset; - if (!isPointerOffset(StartPtr, NextStore->getPointerOperand(), Offset, - DL)) + Optional Offset = + isPointerOffset(StartPtr, NextStore->getPointerOperand(), DL); + if (!Offset) break; - Ranges.addStore(Offset, NextStore); + Ranges.addStore(*Offset, NextStore); } else { MemSetInst *MSI = cast(BI); @@ -349,11 +349,11 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst, break; // Check to see if this store is to a constant offset from the start ptr. - int64_t Offset; - if (!isPointerOffset(StartPtr, MSI->getDest(), Offset, DL)) + Optional Offset = isPointerOffset(StartPtr, MSI->getDest(), DL); + if (!Offset) break; - Ranges.addMemSet(Offset, MSI); + Ranges.addMemSet(*Offset, MSI); } } -- 2.7.4