[llvm] 7392820 - [Align] Remove operations on MaybeAlign that asserted that it had a defined value.
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Fri May 22 23:19:46 PDT 2020
Author: Craig Topper
Date: 2020-05-22T21:54:28-07:00
New Revision: 7392820f989048000562b8fa7dade023dddacb37
URL: https://github.com/llvm/llvm-project/commit/7392820f989048000562b8fa7dade023dddacb37
DIFF: https://github.com/llvm/llvm-project/commit/7392820f989048000562b8fa7dade023dddacb37.diff
LOG: [Align] Remove operations on MaybeAlign that asserted that it had a defined value.
If the caller needs to reponsible for making sure the MaybeAlign
has a value, then we should just make the caller convert it to an Align
with operator*.
I explicitly deleted the relational comparison operators that
were being inherited from Optional. It's unclear what the meaning
of two MaybeAligns were one is defined and the other isn't
should be. So make the caller reponsible for defining the behavior.
I left the ==/!= operators from Optional. But now that exposed a
weird quirk that ==/!= between Align and MaybeAlign required the
MaybeAlign to be defined. But now we use the operator== from
Optional that takes an Optional and the Value.
Differential Revision: https://reviews.llvm.org/D80455
Added:
Modified:
llvm/include/llvm/IR/DataLayout.h
llvm/include/llvm/Support/Alignment.h
llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/Globals.cpp
llvm/lib/LTO/LTO.cpp
llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
llvm/unittests/Support/AlignmentTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index e8fab02fe2ff..ed2336265ab6 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -262,7 +262,7 @@ class DataLayout {
/// Returns true if the given alignment exceeds the natural stack alignment.
bool exceedsNaturalStackAlignment(Align Alignment) const {
- return StackNaturalAlign && (Alignment > StackNaturalAlign);
+ return StackNaturalAlign && (Alignment > *StackNaturalAlign);
}
Align getStackAlignment() const {
diff --git a/llvm/include/llvm/Support/Alignment.h b/llvm/include/llvm/Support/Alignment.h
index ba8ac444f51e..667434e8a407 100644
--- a/llvm/include/llvm/Support/Alignment.h
+++ b/llvm/include/llvm/Support/Alignment.h
@@ -32,8 +32,6 @@ namespace llvm {
#define ALIGN_CHECK_ISPOSITIVE(decl) \
assert(decl > 0 && (#decl " should be defined"))
-#define ALIGN_CHECK_ISSET(decl) \
- assert(decl.hasValue() && (#decl " should be defined"))
/// This struct is a compact representation of a valid (non-zero power of two)
/// alignment.
@@ -151,13 +149,6 @@ inline bool isAligned(Align Lhs, uint64_t SizeInBytes) {
return SizeInBytes % Lhs.value() == 0;
}
-/// Checks that SizeInBytes is a multiple of the alignment.
-/// Returns false if the alignment is undefined.
-inline bool isAligned(MaybeAlign Lhs, uint64_t SizeInBytes) {
- ALIGN_CHECK_ISSET(Lhs);
- return SizeInBytes % (*Lhs).value() == 0;
-}
-
/// Checks that Addr is a multiple of the alignment.
inline bool isAddrAligned(Align Lhs, const void *Addr) {
return isAligned(Lhs, reinterpret_cast<uintptr_t>(Addr));
@@ -225,13 +216,6 @@ inline uint64_t offsetToAlignedAddr(const void *Addr, Align Alignment) {
/// Returns the log2 of the alignment.
inline unsigned Log2(Align A) { return A.ShiftValue; }
-/// Returns the log2 of the alignment.
-/// \pre A must be defined.
-inline unsigned Log2(MaybeAlign A) {
- ALIGN_CHECK_ISSET(A);
- return Log2(A.getValue());
-}
-
/// Returns the alignment that satisfies both alignments.
/// Same semantic as MinAlign.
inline Align commonAlignment(Align A, Align B) { return std::min(A, B); }
@@ -303,26 +287,6 @@ inline bool operator==(MaybeAlign Lhs, uint64_t Rhs) {
inline bool operator!=(MaybeAlign Lhs, uint64_t Rhs) {
return Lhs ? (*Lhs).value() != Rhs : Rhs != 0;
}
-inline bool operator<=(MaybeAlign Lhs, uint64_t Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- ALIGN_CHECK_ISPOSITIVE(Rhs);
- return (*Lhs).value() <= Rhs;
-}
-inline bool operator>=(MaybeAlign Lhs, uint64_t Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- ALIGN_CHECK_ISPOSITIVE(Rhs);
- return (*Lhs).value() >= Rhs;
-}
-inline bool operator<(MaybeAlign Lhs, uint64_t Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- ALIGN_CHECK_ISPOSITIVE(Rhs);
- return (*Lhs).value() < Rhs;
-}
-inline bool operator>(MaybeAlign Lhs, uint64_t Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- ALIGN_CHECK_ISPOSITIVE(Rhs);
- return (*Lhs).value() > Rhs;
-}
/// Comparisons operators between Align.
inline bool operator==(Align Lhs, Align Rhs) {
@@ -344,57 +308,21 @@ inline bool operator>(Align Lhs, Align Rhs) {
return Lhs.ShiftValue > Rhs.ShiftValue;
}
-/// Comparisons operators between Align and MaybeAlign.
-inline bool operator==(Align Lhs, MaybeAlign Rhs) {
- ALIGN_CHECK_ISSET(Rhs);
- return Lhs.value() == (*Rhs).value();
-}
-inline bool operator!=(Align Lhs, MaybeAlign Rhs) {
- ALIGN_CHECK_ISSET(Rhs);
- return Lhs.value() != (*Rhs).value();
-}
-inline bool operator<=(Align Lhs, MaybeAlign Rhs) {
- ALIGN_CHECK_ISSET(Rhs);
- return Lhs.value() <= (*Rhs).value();
-}
-inline bool operator>=(Align Lhs, MaybeAlign Rhs) {
- ALIGN_CHECK_ISSET(Rhs);
- return Lhs.value() >= (*Rhs).value();
-}
-inline bool operator<(Align Lhs, MaybeAlign Rhs) {
- ALIGN_CHECK_ISSET(Rhs);
- return Lhs.value() < (*Rhs).value();
-}
-inline bool operator>(Align Lhs, MaybeAlign Rhs) {
- ALIGN_CHECK_ISSET(Rhs);
- return Lhs.value() > (*Rhs).value();
-}
+// Don't allow relational comparisons with MaybeAlign.
+bool operator<=(Align Lhs, MaybeAlign Rhs) = delete;
+bool operator>=(Align Lhs, MaybeAlign Rhs) = delete;
+bool operator<(Align Lhs, MaybeAlign Rhs) = delete;
+bool operator>(Align Lhs, MaybeAlign Rhs) = delete;
-/// Comparisons operators between MaybeAlign and Align.
-inline bool operator==(MaybeAlign Lhs, Align Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- return Lhs && (*Lhs).value() == Rhs.value();
-}
-inline bool operator!=(MaybeAlign Lhs, Align Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- return Lhs && (*Lhs).value() != Rhs.value();
-}
-inline bool operator<=(MaybeAlign Lhs, Align Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- return Lhs && (*Lhs).value() <= Rhs.value();
-}
-inline bool operator>=(MaybeAlign Lhs, Align Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- return Lhs && (*Lhs).value() >= Rhs.value();
-}
-inline bool operator<(MaybeAlign Lhs, Align Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- return Lhs && (*Lhs).value() < Rhs.value();
-}
-inline bool operator>(MaybeAlign Lhs, Align Rhs) {
- ALIGN_CHECK_ISSET(Lhs);
- return Lhs && (*Lhs).value() > Rhs.value();
-}
+bool operator<=(MaybeAlign Lhs, Align Rhs) = delete;
+bool operator>=(MaybeAlign Lhs, Align Rhs) = delete;
+bool operator<(MaybeAlign Lhs, Align Rhs) = delete;
+bool operator>(MaybeAlign Lhs, Align Rhs) = delete;
+
+bool operator<=(MaybeAlign Lhs, MaybeAlign Rhs) = delete;
+bool operator>=(MaybeAlign Lhs, MaybeAlign Rhs) = delete;
+bool operator<(MaybeAlign Lhs, MaybeAlign Rhs) = delete;
+bool operator>(MaybeAlign Lhs, MaybeAlign Rhs) = delete;
inline Align operator*(Align Lhs, uint64_t Rhs) {
assert(Rhs > 0 && "Rhs must be positive");
@@ -441,7 +369,6 @@ inline std::string DebugStr(const MaybeAlign &MA) {
#endif // NDEBUG
#undef ALIGN_CHECK_ISPOSITIVE
-#undef ALIGN_CHECK_ISSET
} // namespace llvm
diff --git a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
index 7e7ff24f5b8d..5c6e4dd64564 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
@@ -57,7 +57,7 @@ void GISelKnownBits::computeKnownBitsForAlignment(KnownBits &Known,
MaybeAlign Alignment) {
if (Alignment)
// The low bits are known zero if the pointer is aligned.
- Known.Zero.setLowBits(Log2(Alignment));
+ Known.Zero.setLowBits(Log2(*Alignment));
}
KnownBits GISelKnownBits::getKnownBits(MachineInstr &MI) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index ee8a2f90fb94..efdf696f8794 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3906,7 +3906,7 @@ void SelectionDAGBuilder::visitAlloca(const AllocaInst &I) {
// the stack alignment, ignore it. If the size is greater than or equal to
// the stack alignment, we note this in the DYNAMIC_STACKALLOC node.
Align StackAlign = DAG.getSubtarget().getFrameLowering()->getStackAlign();
- if (Alignment <= StackAlign)
+ if (*Alignment <= StackAlign)
Alignment = None;
const uint64_t StackAlignMask = StackAlign.value() - 1U;
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 0137f5285efb..7c1c682d0262 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -121,7 +121,7 @@ unsigned GlobalValue::getAddressSpace() const {
}
void GlobalObject::setAlignment(MaybeAlign Align) {
- assert((!Align || Align <= MaximumAlignment) &&
+ assert((!Align || *Align <= MaximumAlignment) &&
"Alignment is greater than MaximumAlignment!");
unsigned AlignmentData = encode(Align);
unsigned OldData = getGlobalValueSubClassData();
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index ab39a0819250..58fca21a14bf 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -757,8 +757,9 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
// For now they aren't reported correctly by ModuleSymbolTable.
auto &CommonRes = RegularLTO.Commons[std::string(Sym.getIRName())];
CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize());
- CommonRes.Align =
- std::max(CommonRes.Align, MaybeAlign(Sym.getCommonAlignment()));
+ MaybeAlign SymAlign(Sym.getCommonAlignment());
+ if (SymAlign)
+ CommonRes.Align = max(*SymAlign, CommonRes.Align);
CommonRes.Prevailing |= Res.Prevailing;
}
diff --git a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
index 9a55f58e14c9..1ca74bfc3df0 100644
--- a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
@@ -584,8 +584,8 @@ bool ARMTTIImpl::isLegalMaskedLoad(Type *DataTy, MaybeAlign Alignment) {
}
unsigned EltWidth = DataTy->getScalarSizeInBits();
- return (EltWidth == 32 && (!Alignment || Alignment >= 4)) ||
- (EltWidth == 16 && (!Alignment || Alignment >= 2)) ||
+ return (EltWidth == 32 && (!Alignment || *Alignment >= 4)) ||
+ (EltWidth == 16 && (!Alignment || *Alignment >= 2)) ||
(EltWidth == 8);
}
@@ -606,8 +606,8 @@ bool ARMTTIImpl::isLegalMaskedGather(Type *Ty, MaybeAlign Alignment) {
return false;
unsigned EltWidth = Ty->getScalarSizeInBits();
- return ((EltWidth == 32 && (!Alignment || Alignment >= 4)) ||
- (EltWidth == 16 && (!Alignment || Alignment >= 2)) || EltWidth == 8);
+ return ((EltWidth == 32 && (!Alignment || *Alignment >= 4)) ||
+ (EltWidth == 16 && (!Alignment || *Alignment >= 2)) || EltWidth == 8);
}
int ARMTTIImpl::getMemcpyCost(const Instruction *I) {
diff --git a/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp b/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
index b1799c5edde7..002905febbc8 100644
--- a/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
@@ -875,7 +875,7 @@ int PPCTTIImpl::getMemoryOpCost(unsigned Opcode, Type *Src,
// Aligned loads and stores are easy.
unsigned SrcBytes = LT.second.getStoreSize();
- if (!SrcBytes || !Alignment || Alignment >= SrcBytes)
+ if (!SrcBytes || !Alignment || *Alignment >= SrcBytes)
return Cost;
// If we can use the permutation-based load sequence, then this is also
@@ -887,7 +887,7 @@ int PPCTTIImpl::getMemoryOpCost(unsigned Opcode, Type *Src,
// longer true.
if (Opcode == Instruction::Load &&
((!ST->hasP8Vector() && IsAltivecType) || IsQPXType) &&
- Alignment >= LT.second.getScalarType().getStoreSize())
+ *Alignment >= LT.second.getScalarType().getStoreSize())
return Cost + LT.first; // Add the cost of the permutations.
// For VSX, we can do unaligned loads and stores on Altivec/VSX types. On the
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index b0d4406a57ce..94e2e106abc7 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -8198,7 +8198,8 @@ static SDValue LowerAsSplatVectorLoad(SDValue SrcOp, MVT VT, const SDLoc &dl,
SDValue Chain = LD->getChain();
// Make sure the stack object alignment is at least 16 or 32.
MachineFrameInfo &MFI = DAG.getMachineFunction().getFrameInfo();
- if (DAG.InferPtrAlign(Ptr) < RequiredAlign) {
+ MaybeAlign InferredAlign = DAG.InferPtrAlign(Ptr);
+ if (!InferredAlign || *InferredAlign < RequiredAlign) {
if (MFI.isFixedObjectIndex(FI)) {
// Can't change the alignment. FIXME: It's possible to compute
// the exact stack offset and reference FI + adjust offset instead.
@@ -23631,7 +23632,7 @@ X86TargetLowering::LowerDYNAMIC_STACKALLOC(SDValue Op,
Chain = SP.getValue(1);
Result = DAG.getNode(ISD::SUB, dl, VT, SP, Size); // Value
}
- if (Alignment && Alignment > StackAlign)
+ if (Alignment && *Alignment > StackAlign)
Result =
DAG.getNode(ISD::AND, dl, VT, Result,
DAG.getConstant(~(Alignment->value() - 1ULL), dl, VT));
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index ff7b23a00d5e..6b3876ed91aa 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -3565,7 +3565,7 @@ static unsigned getKnownAlignForUse(Attributor &A,
MA = LI->getAlign();
}
- if (!MA.hasValue() || MA <= 1)
+ if (!MA || *MA <= 1)
return 0;
unsigned Alignment = MA->value();
diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index 33dd0de3b67c..73144953ccd4 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -1251,9 +1251,10 @@ bool MemCpyOptPass::processByValArgument(CallBase &CB, unsigned ArgNo) {
// source of the memcpy to the alignment we need. If we fail, we bail out.
AssumptionCache &AC = LookupAssumptionCache();
DominatorTree &DT = LookupDomTree();
- if (MDep->getSourceAlign() < ByValAlign &&
+ MaybeAlign MemDepAlign = MDep->getSourceAlign();
+ if ((!MemDepAlign || *MemDepAlign < *ByValAlign) &&
getOrEnforceKnownAlignment(MDep->getSource(), ByValAlign, DL, &CB, &AC,
- &DT) < ByValAlign)
+ &DT) < *ByValAlign)
return false;
// The address space of the memcpy source must match the byval argument
diff --git a/llvm/unittests/Support/AlignmentTest.cpp b/llvm/unittests/Support/AlignmentTest.cpp
index 58f3fb100bbd..5ef9cd4b2ac7 100644
--- a/llvm/unittests/Support/AlignmentTest.cpp
+++ b/llvm/unittests/Support/AlignmentTest.cpp
@@ -135,7 +135,6 @@ TEST(AlignmentTest, AlignToWithSkew) {
TEST(AlignmentTest, Log2) {
for (uint64_t Value : getValidAlignments()) {
EXPECT_EQ(Log2(Align(Value)), Log2_64(Value));
- EXPECT_EQ(Log2(MaybeAlign(Value)), Log2_64(Value));
}
}
@@ -203,8 +202,6 @@ TEST(AlignmentTest, isAligned_isAddrAligned) {
};
for (const auto &T : kTests) {
MaybeAlign A(T.alignment);
- // Test MaybeAlign
- EXPECT_EQ(isAligned(A, T.offset), T.isAligned);
// Test Align
if (A) {
EXPECT_EQ(isAligned(A.getValue(), T.offset), T.isAligned);
@@ -266,21 +263,9 @@ TEST(AlignmentTest, AlignComparisons) {
const MaybeAlign MB(ValidAlignments[I]);
EXPECT_EQ(MA, MA);
EXPECT_NE(MA, MB);
- EXPECT_LT(MA, MB);
- EXPECT_GT(MB, MA);
- EXPECT_LE(MA, MB);
- EXPECT_GE(MB, MA);
- EXPECT_LE(MA, MA);
- EXPECT_GE(MA, MA);
EXPECT_EQ(MA, MA ? (*MA).value() : 0);
EXPECT_NE(MA, MB ? (*MB).value() : 0);
- EXPECT_LT(MA, MB ? (*MB).value() : 0);
- EXPECT_GT(MB, MA ? (*MA).value() : 0);
- EXPECT_LE(MA, MB ? (*MB).value() : 0);
- EXPECT_GE(MB, MA ? (*MA).value() : 0);
- EXPECT_LE(MA, MA ? (*MA).value() : 0);
- EXPECT_GE(MA, MA ? (*MA).value() : 0);
EXPECT_EQ(std::max(A, B), B);
EXPECT_EQ(std::min(A, B), A);
@@ -306,8 +291,6 @@ TEST(AlignmentTest, Max) {
// Uses std::max.
EXPECT_EQ(max(Align(2), Align(4)), Align(4));
- EXPECT_EQ(max(MaybeAlign(2), MaybeAlign(4)), MaybeAlign(4));
- EXPECT_EQ(max(MaybeAlign(), MaybeAlign()), MaybeAlign());
}
TEST(AlignmentTest, AssumeAligned) {
@@ -328,10 +311,6 @@ std::vector<uint64_t> getValidAlignmentsForDeathTest() {
std::vector<uint64_t> getNonPowerOfTwo() { return {3, 10, 15}; }
-TEST(AlignmentDeathTest, Log2) {
- EXPECT_DEATH(Log2(MaybeAlign(0)), ".* should be defined");
-}
-
TEST(AlignmentDeathTest, CantConvertUnsetMaybe) {
EXPECT_DEATH((MaybeAlign(0).getValue()), ".*");
}
@@ -369,21 +348,6 @@ TEST(AlignmentDeathTest, CompareMaybeAlignToZero) {
// MaybeAlign is allowed to be == or != 0
(void)(MaybeAlign(Value) == 0);
(void)(MaybeAlign(Value) != 0);
- EXPECT_DEATH((void)(MaybeAlign(Value) >= 0), ".* should be defined");
- EXPECT_DEATH((void)(MaybeAlign(Value) <= 0), ".* should be defined");
- EXPECT_DEATH((void)(MaybeAlign(Value) > 0), ".* should be defined");
- EXPECT_DEATH((void)(MaybeAlign(Value) < 0), ".* should be defined");
- }
-}
-
-TEST(AlignmentDeathTest, CompareAlignToUndefMaybeAlign) {
- for (uint64_t Value : getValidAlignmentsForDeathTest()) {
- EXPECT_DEATH((void)(Align(Value) == MaybeAlign(0)), ".* should be defined");
- EXPECT_DEATH((void)(Align(Value) != MaybeAlign(0)), ".* should be defined");
- EXPECT_DEATH((void)(Align(Value) >= MaybeAlign(0)), ".* should be defined");
- EXPECT_DEATH((void)(Align(Value) <= MaybeAlign(0)), ".* should be defined");
- EXPECT_DEATH((void)(Align(Value) > MaybeAlign(0)), ".* should be defined");
- EXPECT_DEATH((void)(Align(Value) < MaybeAlign(0)), ".* should be defined");
}
}
More information about the llvm-commits
mailing list