[ValueTracking] Use SimplifyQuery in some public APIs (NFC) #68290
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms ChangesThis patch moves SimplifyQuery into a separate header, so that both InstSimplify and ValueTracking can use it in their public API. ValueTracking already (mostly) uses SimplifyQuery for its internal recursive calls, but the public API accepts unpacked arguments. I'd like to move things towards accepting SimplifyQuery in the public API as well. This patch in particular migrates the computeOverflowXYZ() APIs. Patch is 31.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/68290.diff 8 Files Affected:
diff --git a/llvm/include/llvm/Analysis/InstructionSimplify.h b/llvm/include/llvm/Analysis/InstructionSimplify.h
index 92005da1f4c61e7..c626a6522d01779 100644
--- a/llvm/include/llvm/Analysis/InstructionSimplify.h
+++ b/llvm/include/llvm/Analysis/InstructionSimplify.h
@@ -31,7 +31,7 @@
#ifndef LLVM_ANALYSIS_INSTRUCTIONSIMPLIFY_H
#define LLVM_ANALYSIS_INSTRUCTIONSIMPLIFY_H
-#include "llvm/IR/PatternMatch.h"
+#include "llvm/Analysis/SimplifyQuery.h"
namespace llvm {
@@ -52,102 +52,6 @@ class TargetLibraryInfo;
class Type;
class Value;
-/// InstrInfoQuery provides an interface to query additional information for
-/// instructions like metadata or keywords like nsw, which provides conservative
-/// results if the users specified it is safe to use.
-struct InstrInfoQuery {
- InstrInfoQuery(bool UMD) : UseInstrInfo(UMD) {}
- InstrInfoQuery() = default;
- bool UseInstrInfo = true;
-
- MDNode *getMetadata(const Instruction *I, unsigned KindID) const {
- if (UseInstrInfo)
- return I->getMetadata(KindID);
- return nullptr;
- }
-
- template <class InstT> bool hasNoUnsignedWrap(const InstT *Op) const {
- if (UseInstrInfo)
- return Op->hasNoUnsignedWrap();
- return false;
- }
-
- template <class InstT> bool hasNoSignedWrap(const InstT *Op) const {
- if (UseInstrInfo)
- return Op->hasNoSignedWrap();
- return false;
- }
-
- bool isExact(const BinaryOperator *Op) const {
- if (UseInstrInfo && isa<PossiblyExactOperator>(Op))
- return cast<PossiblyExactOperator>(Op)->isExact();
- return false;
- }
-
- template <class InstT> bool hasNoSignedZeros(const InstT *Op) const {
- if (UseInstrInfo)
- return Op->hasNoSignedZeros();
- return false;
- }
-};
-
-struct SimplifyQuery {
- const DataLayout &DL;
- const TargetLibraryInfo *TLI = nullptr;
- const DominatorTree *DT = nullptr;
- AssumptionCache *AC = nullptr;
- const Instruction *CxtI = nullptr;
-
- // Wrapper to query additional information for instructions like metadata or
- // keywords like nsw, which provides conservative results if those cannot
- // be safely used.
- const InstrInfoQuery IIQ;
-
- /// Controls whether simplifications are allowed to constrain the range of
- /// possible values for uses of undef. If it is false, simplifications are not
- /// allowed to assume a particular value for a use of undef for example.
- bool CanUseUndef = true;
-
- SimplifyQuery(const DataLayout &DL, const Instruction *CXTI = nullptr)
- : DL(DL), CxtI(CXTI) {}
-
- SimplifyQuery(const DataLayout &DL, const TargetLibraryInfo *TLI,
- const DominatorTree *DT = nullptr,
- AssumptionCache *AC = nullptr,
- const Instruction *CXTI = nullptr, bool UseInstrInfo = true,
- bool CanUseUndef = true)
- : DL(DL), TLI(TLI), DT(DT), AC(AC), CxtI(CXTI), IIQ(UseInstrInfo),
- CanUseUndef(CanUseUndef) {}
-
- SimplifyQuery(const DataLayout &DL, const DominatorTree *DT,
- AssumptionCache *AC = nullptr,
- const Instruction *CXTI = nullptr, bool UseInstrInfo = true,
- bool CanUseUndef = true)
- : DL(DL), DT(DT), AC(AC), CxtI(CXTI), IIQ(UseInstrInfo),
- CanUseUndef(CanUseUndef) {}
-
- SimplifyQuery getWithInstruction(const Instruction *I) const {
- SimplifyQuery Copy(*this);
- Copy.CxtI = I;
- return Copy;
- }
- SimplifyQuery getWithoutUndef() const {
- SimplifyQuery Copy(*this);
- Copy.CanUseUndef = false;
- return Copy;
- }
-
- /// If CanUseUndef is true, returns whether \p V is undef.
- /// Otherwise always return false.
- bool isUndefValue(Value *V) const {
- if (!CanUseUndef)
- return false;
-
- using namespace PatternMatch;
- return match(V, m_Undef());
- }
-};
-
// NOTE: the explicit multiple argument versions of these functions are
// deprecated.
// Please use the SimplifyQuery versions in new code.
diff --git a/llvm/include/llvm/Analysis/SimplifyQuery.h b/llvm/include/llvm/Analysis/SimplifyQuery.h
new file mode 100644
index 000000000000000..f9cc3029221d679
--- /dev/null
+++ b/llvm/include/llvm/Analysis/SimplifyQuery.h
@@ -0,0 +1,118 @@
+//===-- SimplifyQuery.h - Context for simplifications -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ANALYSIS_SIMPLIFYQUERY_H
+#define LLVM_ANALYSIS_SIMPLIFYQUERY_H
+
+#include "llvm/IR/PatternMatch.h"
+
+namespace llvm {
+
+class AssumptionCache;
+class DominatorTree;
+class TargetLibraryInfo;
+
+/// InstrInfoQuery provides an interface to query additional information for
+/// instructions like metadata or keywords like nsw, which provides conservative
+/// results if the users specified it is safe to use.
+struct InstrInfoQuery {
+ InstrInfoQuery(bool UMD) : UseInstrInfo(UMD) {}
+ InstrInfoQuery() = default;
+ bool UseInstrInfo = true;
+
+ MDNode *getMetadata(const Instruction *I, unsigned KindID) const {
+ if (UseInstrInfo)
+ return I->getMetadata(KindID);
+ return nullptr;
+ }
+
+ template <class InstT> bool hasNoUnsignedWrap(const InstT *Op) const {
+ if (UseInstrInfo)
+ return Op->hasNoUnsignedWrap();
+ return false;
+ }
+
+ template <class InstT> bool hasNoSignedWrap(const InstT *Op) const {
+ if (UseInstrInfo)
+ return Op->hasNoSignedWrap();
+ return false;
+ }
+
+ bool isExact(const BinaryOperator *Op) const {
+ if (UseInstrInfo && isa<PossiblyExactOperator>(Op))
+ return cast<PossiblyExactOperator>(Op)->isExact();
+ return false;
+ }
+
+ template <class InstT> bool hasNoSignedZeros(const InstT *Op) const {
+ if (UseInstrInfo)
+ return Op->hasNoSignedZeros();
+ return false;
+ }
+};
+
+struct SimplifyQuery {
+ const DataLayout &DL;
+ const TargetLibraryInfo *TLI = nullptr;
+ const DominatorTree *DT = nullptr;
+ AssumptionCache *AC = nullptr;
+ const Instruction *CxtI = nullptr;
+
+ // Wrapper to query additional information for instructions like metadata or
+ // keywords like nsw, which provides conservative results if those cannot
+ // be safely used.
+ const InstrInfoQuery IIQ;
+
+ /// Controls whether simplifications are allowed to constrain the range of
+ /// possible values for uses of undef. If it is false, simplifications are not
+ /// allowed to assume a particular value for a use of undef for example.
+ bool CanUseUndef = true;
+
+ SimplifyQuery(const DataLayout &DL, const Instruction *CXTI = nullptr)
+ : DL(DL), CxtI(CXTI) {}
+
+ SimplifyQuery(const DataLayout &DL, const TargetLibraryInfo *TLI,
+ const DominatorTree *DT = nullptr,
+ AssumptionCache *AC = nullptr,
+ const Instruction *CXTI = nullptr, bool UseInstrInfo = true,
+ bool CanUseUndef = true)
+ : DL(DL), TLI(TLI), DT(DT), AC(AC), CxtI(CXTI), IIQ(UseInstrInfo),
+ CanUseUndef(CanUseUndef) {}
+
+ SimplifyQuery(const DataLayout &DL, const DominatorTree *DT,
+ AssumptionCache *AC = nullptr,
+ const Instruction *CXTI = nullptr, bool UseInstrInfo = true,
+ bool CanUseUndef = true)
+ : DL(DL), DT(DT), AC(AC), CxtI(CXTI), IIQ(UseInstrInfo),
+ CanUseUndef(CanUseUndef) {}
+
+ SimplifyQuery getWithInstruction(const Instruction *I) const {
+ SimplifyQuery Copy(*this);
+ Copy.CxtI = I;
+ return Copy;
+ }
+ SimplifyQuery getWithoutUndef() const {
+ SimplifyQuery Copy(*this);
+ Copy.CanUseUndef = false;
+ return Copy;
+ }
+
+ /// If CanUseUndef is true, returns whether \p V is undef.
+ /// Otherwise always return false.
+ bool isUndefValue(Value *V) const {
+ if (!CanUseUndef)
+ return false;
+
+ using namespace PatternMatch;
+ return match(V, m_Undef());
+ }
+};
+
+} // end namespace llvm
+
+#endif
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index a2f5f23d94ee812..5778df32ef1b8cd 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -16,6 +16,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallSet.h"
+#include "llvm/Analysis/SimplifyQuery.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/FMF.h"
@@ -39,7 +40,6 @@ struct KnownBits;
class Loop;
class LoopInfo;
class MDNode;
-struct SimplifyQuery;
class StringRef;
class TargetLibraryInfo;
class Value;
@@ -851,44 +851,20 @@ enum class OverflowResult {
};
OverflowResult computeOverflowForUnsignedMul(const Value *LHS, const Value *RHS,
- const DataLayout &DL,
- AssumptionCache *AC,
- const Instruction *CxtI,
- const DominatorTree *DT,
- bool UseInstrInfo = true);
+ const SimplifyQuery &SQ);
OverflowResult computeOverflowForSignedMul(const Value *LHS, const Value *RHS,
- const DataLayout &DL,
- AssumptionCache *AC,
- const Instruction *CxtI,
- const DominatorTree *DT,
- bool UseInstrInfo = true);
+ const SimplifyQuery &SQ);
OverflowResult computeOverflowForUnsignedAdd(const Value *LHS, const Value *RHS,
- const DataLayout &DL,
- AssumptionCache *AC,
- const Instruction *CxtI,
- const DominatorTree *DT,
- bool UseInstrInfo = true);
+ const SimplifyQuery &SQ);
OverflowResult computeOverflowForSignedAdd(const Value *LHS, const Value *RHS,
- const DataLayout &DL,
- AssumptionCache *AC = nullptr,
- const Instruction *CxtI = nullptr,
- const DominatorTree *DT = nullptr);
+ const SimplifyQuery &SQ);
/// This version also leverages the sign bit of Add if known.
OverflowResult computeOverflowForSignedAdd(const AddOperator *Add,
- const DataLayout &DL,
- AssumptionCache *AC = nullptr,
- const Instruction *CxtI = nullptr,
- const DominatorTree *DT = nullptr);
+ const SimplifyQuery &SQ);
OverflowResult computeOverflowForUnsignedSub(const Value *LHS, const Value *RHS,
- const DataLayout &DL,
- AssumptionCache *AC,
- const Instruction *CxtI,
- const DominatorTree *DT);
+ const SimplifyQuery &SQ);
OverflowResult computeOverflowForSignedSub(const Value *LHS, const Value *RHS,
- const DataLayout &DL,
- AssumptionCache *AC,
- const Instruction *CxtI,
- const DominatorTree *DT);
+ const SimplifyQuery &SQ);
/// Returns true if the arithmetic part of the \p WO 's result is
/// used only along the paths control dependent on the computation
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index c1cea5649f769ab..dcfcc8f41dd58d0 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -500,34 +500,40 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
OverflowResult computeOverflowForUnsignedMul(const Value *LHS,
const Value *RHS,
const Instruction *CxtI) const {
- return llvm::computeOverflowForUnsignedMul(LHS, RHS, DL, &AC, CxtI, &DT);
+ return llvm::computeOverflowForUnsignedMul(LHS, RHS,
+ SQ.getWithInstruction(CxtI));
}
OverflowResult computeOverflowForSignedMul(const Value *LHS, const Value *RHS,
const Instruction *CxtI) const {
- return llvm::computeOverflowForSignedMul(LHS, RHS, DL, &AC, CxtI, &DT);
+ return llvm::computeOverflowForSignedMul(LHS, RHS,
+ SQ.getWithInstruction(CxtI));
}
OverflowResult computeOverflowForUnsignedAdd(const Value *LHS,
const Value *RHS,
const Instruction *CxtI) const {
- return llvm::computeOverflowForUnsignedAdd(LHS, RHS, DL, &AC, CxtI, &DT);
+ return llvm::computeOverflowForUnsignedAdd(LHS, RHS,
+ SQ.getWithInstruction(CxtI));
}
OverflowResult computeOverflowForSignedAdd(const Value *LHS, const Value *RHS,
const Instruction *CxtI) const {
- return llvm::computeOverflowForSignedAdd(LHS, RHS, DL, &AC, CxtI, &DT);
+ return llvm::computeOverflowForSignedAdd(LHS, RHS,
+ SQ.getWithInstruction(CxtI));
}
OverflowResult computeOverflowForUnsignedSub(const Value *LHS,
const Value *RHS,
const Instruction *CxtI) const {
- return llvm::computeOverflowForUnsignedSub(LHS, RHS, DL, &AC, CxtI, &DT);
+ return llvm::computeOverflowForUnsignedSub(LHS, RHS,
+ SQ.getWithInstruction(CxtI));
}
OverflowResult computeOverflowForSignedSub(const Value *LHS, const Value *RHS,
const Instruction *CxtI) const {
- return llvm::computeOverflowForSignedSub(LHS, RHS, DL, &AC, CxtI, &DT);
+ return llvm::computeOverflowForSignedSub(LHS, RHS,
+ SQ.getWithInstruction(CxtI));
}
virtual bool SimplifyDemandedBits(Instruction *I, unsigned OpNo,
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index b76becf24d10fc9..ad55698c7c19025 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -6245,37 +6245,30 @@ static OverflowResult mapOverflowResult(ConstantRange::OverflowResult OR) {
}
/// Combine constant ranges from computeConstantRange() and computeKnownBits().
-static ConstantRange computeConstantRangeIncludingKnownBits(
- const Value *V, bool ForSigned, const DataLayout &DL, AssumptionCache *AC,
- const Instruction *CxtI, const DominatorTree *DT,
- bool UseInstrInfo = true) {
- KnownBits Known =
- computeKnownBits(V, DL, /*Depth=*/0, AC, CxtI, DT, UseInstrInfo);
+static ConstantRange
+computeConstantRangeIncludingKnownBits(const Value *V, bool ForSigned,
+ const SimplifyQuery &SQ) {
+ KnownBits Known = ::computeKnownBits(V, /*Depth=*/0, SQ);
ConstantRange CR1 = ConstantRange::fromKnownBits(Known, ForSigned);
- ConstantRange CR2 = computeConstantRange(V, ForSigned, UseInstrInfo);
+ ConstantRange CR2 = computeConstantRange(V, ForSigned, SQ.IIQ.UseInstrInfo);
ConstantRange::PreferredRangeType RangeType =
ForSigned ? ConstantRange::Signed : ConstantRange::Unsigned;
return CR1.intersectWith(CR2, RangeType);
}
-OverflowResult llvm::computeOverflowForUnsignedMul(
- const Value *LHS, const Value *RHS, const DataLayout &DL,
- AssumptionCache *AC, const Instruction *CxtI, const DominatorTree *DT,
- bool UseInstrInfo) {
- KnownBits LHSKnown = computeKnownBits(LHS, DL, /*Depth=*/0, AC, CxtI, DT,
- UseInstrInfo);
- KnownBits RHSKnown = computeKnownBits(RHS, DL, /*Depth=*/0, AC, CxtI, DT,
- UseInstrInfo);
+OverflowResult llvm::computeOverflowForUnsignedMul(const Value *LHS,
+ const Value *RHS,
+ const SimplifyQuery &SQ) {
+ KnownBits LHSKnown = ::computeKnownBits(LHS, /*Depth=*/0, SQ);
+ KnownBits RHSKnown = ::computeKnownBits(RHS, /*Depth=*/0, SQ);
ConstantRange LHSRange = ConstantRange::fromKnownBits(LHSKnown, false);
ConstantRange RHSRange = ConstantRange::fromKnownBits(RHSKnown, false);
return mapOverflowResult(LHSRange.unsignedMulMayOverflow(RHSRange));
}
-OverflowResult
-llvm::computeOverflowForSignedMul(const Value *LHS, const Value *RHS,
- const DataLayout &DL, AssumptionCache *AC,
- const Instruction *CxtI,
- const DominatorTree *DT, bool UseInstrInfo) {
+OverflowResult llvm::computeOverflowForSignedMul(const Value *LHS,
+ const Value *RHS,
+ const SimplifyQuery &SQ) {
// Multiplying n * m significant bits yields a result of n + m significant
// bits. If the total number of significant bits does not exceed the
// result bit width (minus 1), there is no overflow.
@@ -6286,8 +6279,8 @@ llvm::computeOverflowForSignedMul(const Value *LHS, const Value *RHS,
// Note that underestimating the number of sign bits gives a more
// conservative answer.
- unsigned SignBits = ComputeNumSignBits(LHS, DL, 0, AC, CxtI, DT) +
- ComputeNumSignBits(RHS, DL, 0, AC, CxtI, DT);
+ unsigned SignBits =
+ ::ComputeNumSignBits(LHS, 0, SQ) + ::ComputeNumSignBits(RHS, 0, SQ);
// First handle the easy case: if we have enough sign bits there's
// definitely no overflow.
@@ -6304,34 +6297,28 @@ llvm::computeOverflowForSignedMul(const Value *LHS, const Value *RHS,
// product is exactly the minimum negative number.
// E.g. mul i16 with 17 sign bits: 0xff00 * 0xff80 = 0x8000
// For simplicity we just check if at least one side is not negative.
- KnownBits LHSKnown = computeKnownBits(LHS, DL, /*Depth=*/0, AC, CxtI, DT,
- UseInstrInfo);
- KnownBits RHSKnown = computeKnownBits(RHS, DL, /*Depth=*/0, AC, CxtI, DT,
- UseInstrInfo);
+ KnownBits LHSKnown = ::computeKnownBits(LHS, /*Depth=*/0, SQ);
+ KnownBits RHSKnown = ::computeKnownBits(RHS, /*Depth=*/0, SQ);
if (LHSKnown.isNonNegative() || RHSKnown.isNonNegative())
return OverflowResult::NeverOverflows;
}
return OverflowResult::MayOverflow;
}
-OverflowResult llvm::computeOverflowForUnsignedAdd(
- const Value *LHS, const Value *RHS, const DataLayout &DL,
- AssumptionCache *AC, const Instruction *CxtI, const DominatorTree *DT,
- bool UseInstrInfo) {
- ConstantRange LHSRange = computeConstantRangeIncludingKnownBits(
- LHS, /*ForSigned=*/false, DL, AC, CxtI, DT, UseInstrInfo);
- ConstantRange RHSRange = computeConstantRangeIncludingKnownBits(
- RHS, /*ForSigned=*/false, DL, AC, CxtI, DT, UseInstrInfo);
+OverflowResult llvm::computeOverflowForUnsignedAdd(const Value *LHS,
+ const Value *RHS,
+ const SimplifyQuery &SQ) {
+ ConstantRange LHSRange =
+ computeConstantRangeIncludingKnownBits(LHS, /*ForSigned=*/false, SQ);
+ ConstantRange RHSRange =
+ computeConstantRangeIncludingKnownBits(RHS, /*ForSigned=*/false, SQ);
retur...
[truncated]
|
fhahn
approved these changes
Oct 5, 2023
| @@ -6245,37 +6245,30 @@ static OverflowResult mapOverflowResult(ConstantRange::OverflowResult OR) { | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Should this explicitly include SimplifyQuery.h?
| @@ -39,7 +40,6 @@ struct KnownBits; | |||
| class Loop; | |||
| class LoopInfo; | |||
| class MDNode; | |||
| struct SimplifyQuery; | |||
There was a problem hiding this comment.
Can the forward declaration be retained instead of including the header here?
There was a problem hiding this comment.
This is possible, but my thinking here was that (at least after more APIs have been migrated) actually using any ValueTracking functions will need SimplifyQuery.h anyway, so it would be more convenient if ValueTracking.h directly pulls it in, instead of including it in each user.
Including it also allows using the implicit DataLayout -> SimplifyQuery conversion, though I'm not sure how much people use that.
There was a problem hiding this comment.
Does LLVM try to adhere to pattern of including minimum number of dependent headers that leaves code compiling? If so would say fwd declare makes more sense.
Otherwise think most people pulling in ValueTracking will also want simplifyquery so leaving in makes sense.
There was a problem hiding this comment.
I think in general we often try to limit includes in headers if forward declarations are sufficient. I don't have any strong feelings about this case, if it is overall beneficial to include it in the header that would be fine.
There was a problem hiding this comment.
Okay, if nobody feels strongly about this, I'd prefer to keep the current version.
| @@ -500,34 +500,40 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner { | |||
| OverflowResult computeOverflowForUnsignedMul(const Value *LHS, | |||
| const Value *RHS, | |||
| const Instruction *CxtI) const { | |||
| return llvm::computeOverflowForUnsignedMul(LHS, RHS, DL, &AC, CxtI, &DT); | |||
| return llvm::computeOverflowForUnsignedMul(LHS, RHS, | |||
| SQ.getWithInstruction(CxtI)); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Should this explicitly include SimplifyQuery.h?
| @@ -2540,8 +2540,9 @@ static bool hoistAdd(ICmpInst::Predicate Pred, Value *VariantLHS, | |||
| // we want to avoid this. | |||
There was a problem hiding this comment.
Should this explicitly include SimplifyQuery.h?
| @@ -641,8 +641,9 @@ static OverflowResult checkOverflow(FlattenInfo &FI, DominatorTree *DT, | |||
| // Check if the multiply could not overflow due to known ranges of the | |||
There was a problem hiding this comment.
Should this explicitly include SimplifyQuery.h?
| @@ -372,9 +372,9 @@ NaryReassociatePass::tryReassociateGEPAtIndex(GetElementPtrInst *GEP, | |||
| // If the I-th index needs sext and the underlying add is not equipped with | |||
There was a problem hiding this comment.
Should this explicitly include SimplifyQuery.h?
goldsteinn
reviewed
Oct 9, 2023
| @@ -6407,32 +6390,29 @@ OverflowResult llvm::computeOverflowForUnsignedSub(const Value *LHS, | |||
| // See simplifyICmpWithBinOpOnLHS() for candidates. | |||
| if (match(RHS, m_URem(m_Specific(LHS), m_Value())) || | |||
| match(RHS, m_NUWSub(m_Specific(LHS), m_Value()))) | |||
| if (isGuaranteedNotToBeUndefOrPoison(LHS, AC, CxtI, DT)) | |||
| if (isGuaranteedNotToBeUndefOrPoison(LHS, SQ.AC, SQ.CxtI, SQ.DT)) | |||
There was a problem hiding this comment.
Should isGuranteedNotToBeUndefOrPoison be made to take SQ as well?
There was a problem hiding this comment.
Yeah, I'm just starting out with some of the lesser used functions here.
goldsteinn
reviewed
Oct 9, 2023
| m_Intrinsic<Intrinsic::usub_with_overflow>(m_Value(), m_Value()))) | ||
| if (auto C = | ||
| isImpliedByDomCondition(CmpInst::ICMP_UGE, LHS, RHS, CxtI, DL)) { | ||
| if (auto C = isImpliedByDomCondition(CmpInst::ICMP_UGE, LHS, RHS, SQ.CxtI, |
There was a problem hiding this comment.
likewise isImpliedByDomCondition.
To allow reusing it between InstructionSimplify and ValueTracking.
Accept a SimplifyQuery instead of an unpacked list of arguments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch moves SimplifyQuery into a separate header, so that both InstSimplify and ValueTracking can use it in their public API. ValueTracking already (mostly) uses SimplifyQuery for its internal recursive calls, but the public API accepts unpacked arguments.
I'd like to move things towards accepting SimplifyQuery in the public API as well. This patch in particular migrates the computeOverflowXYZ() APIs.