[llvm] 2c5ff48 - [Alignment][NFC] Migrate AtomicExpandPass to Align
Guillaume Chatelet via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 30 02:55:01 PDT 2020
Author: Guillaume Chatelet
Date: 2020-06-30T09:54:45Z
New Revision: 2c5ff48e61b5611dee5696c103e654c98d7e43f1
URL: https://github.com/llvm/llvm-project/commit/2c5ff48e61b5611dee5696c103e654c98d7e43f1
DIFF: https://github.com/llvm/llvm-project/commit/2c5ff48e61b5611dee5696c103e654c98d7e43f1.diff
LOG: [Alignment][NFC] Migrate AtomicExpandPass to Align
This is a followup on D78403.
I'm unsure about `getAtomicOpAlign` overloads that take `AtomicRMWInst` and `AtomicCmpXchgInst`, shouldn't `getAlign` provide the correct answer already?
Differential Revision: https://reviews.llvm.org/D81369
Added:
Modified:
llvm/include/llvm/IR/Instructions.h
llvm/lib/CodeGen/AtomicExpandPass.cpp
llvm/lib/IR/Instructions.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index 7710542efd95..caa24f4b1a78 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -525,6 +525,11 @@ class AtomicCmpXchgInst : public Instruction {
return User::operator new(s, 3);
}
+ /// Always returns the natural type alignment.
+ /// FIXME: Introduce a proper alignment
+ /// https://bugs.llvm.org/show_bug.cgi?id=27168
+ Align getAlign() const;
+
/// Return true if this is a cmpxchg from a volatile memory
/// location.
///
@@ -743,6 +748,11 @@ class AtomicRMWInst : public Instruction {
(Operation << 5));
}
+ /// Always returns the natural type alignment.
+ /// FIXME: Introduce a proper alignment
+ /// https://bugs.llvm.org/show_bug.cgi?id=27168
+ Align getAlign() const;
+
/// Return true if this is a RMW on a volatile memory location.
///
bool isVolatile() const {
diff --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index 9bec110604d9..2b51e8c84a53 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -105,7 +105,7 @@ namespace {
bool isIdempotentRMW(AtomicRMWInst *RMWI);
bool simplifyIdempotentRMW(AtomicRMWInst *RMWI);
- bool expandAtomicOpToLibcall(Instruction *I, unsigned Size, unsigned Align,
+ bool expandAtomicOpToLibcall(Instruction *I, unsigned Size, Align Alignment,
Value *PointerOperand, Value *ValueOperand,
Value *CASExpected, AtomicOrdering Ordering,
AtomicOrdering Ordering2,
@@ -152,47 +152,15 @@ static unsigned getAtomicOpSize(AtomicCmpXchgInst *CASI) {
return DL.getTypeStoreSize(CASI->getCompareOperand()->getType());
}
-// Helper functions to retrieve the alignment of atomic instructions.
-static unsigned getAtomicOpAlign(LoadInst *LI) {
- unsigned Align = LI->getAlignment();
- // In the future, if this IR restriction is relaxed, we should
- // return DataLayout::getABITypeAlignment when there's no align
- // value.
- assert(Align != 0 && "An atomic LoadInst always has an explicit alignment");
- return Align;
-}
-
-static unsigned getAtomicOpAlign(StoreInst *SI) {
- unsigned Align = SI->getAlignment();
- // In the future, if this IR restriction is relaxed, we should
- // return DataLayout::getABITypeAlignment when there's no align
- // value.
- assert(Align != 0 && "An atomic StoreInst always has an explicit alignment");
- return Align;
-}
-
-static unsigned getAtomicOpAlign(AtomicRMWInst *RMWI) {
- // TODO(PR27168): This instruction has no alignment attribute, but unlike the
- // default alignment for load/store, the default here is to assume
- // it has NATURAL alignment, not DataLayout-specified alignment.
- const DataLayout &DL = RMWI->getModule()->getDataLayout();
- return DL.getTypeStoreSize(RMWI->getValOperand()->getType());
-}
-
-static unsigned getAtomicOpAlign(AtomicCmpXchgInst *CASI) {
- // TODO(PR27168): same comment as above.
- const DataLayout &DL = CASI->getModule()->getDataLayout();
- return DL.getTypeStoreSize(CASI->getCompareOperand()->getType());
-}
-
// Determine if a particular atomic operation has a supported size,
// and is of appropriate alignment, to be passed through for target
// lowering. (Versus turning into a __atomic libcall)
template <typename Inst>
static bool atomicSizeSupported(const TargetLowering *TLI, Inst *I) {
unsigned Size = getAtomicOpSize(I);
- unsigned Align = getAtomicOpAlign(I);
- return Align >= Size && Size <= TLI->getMaxAtomicSizeInBitsSupported() / 8;
+ Align Alignment = I->getAlign();
+ return Alignment >= Size &&
+ Size <= TLI->getMaxAtomicSizeInBitsSupported() / 8;
}
bool AtomicExpand::runOnFunction(Function &F) {
@@ -1513,7 +1481,7 @@ bool llvm::expandAtomicRMWToCmpXchg(AtomicRMWInst *AI,
// must be one of the potentially-specialized sizes, and the value
// type must actually exist in C on the target (otherwise, the
// function wouldn't actually be defined.)
-static bool canUseSizedAtomicCall(unsigned Size, unsigned Align,
+static bool canUseSizedAtomicCall(unsigned Size, Align Alignment,
const DataLayout &DL) {
// TODO: "LargestSize" is an approximation for "largest type that
// you can express in C". It seems to be the case that int128 is
@@ -1523,7 +1491,7 @@ static bool canUseSizedAtomicCall(unsigned Size, unsigned Align,
// really be some more reliable way in LLVM of determining integer
// sizes which are valid in the target's C ABI...
unsigned LargestSize = DL.getLargestLegalIntTypeSizeInBits() >= 64 ? 16 : 8;
- return Align >= Size &&
+ return Alignment >= Size &&
(Size == 1 || Size == 2 || Size == 4 || Size == 8 || Size == 16) &&
Size <= LargestSize;
}
@@ -1533,10 +1501,9 @@ void AtomicExpand::expandAtomicLoadToLibcall(LoadInst *I) {
RTLIB::ATOMIC_LOAD, RTLIB::ATOMIC_LOAD_1, RTLIB::ATOMIC_LOAD_2,
RTLIB::ATOMIC_LOAD_4, RTLIB::ATOMIC_LOAD_8, RTLIB::ATOMIC_LOAD_16};
unsigned Size = getAtomicOpSize(I);
- unsigned Align = getAtomicOpAlign(I);
bool expanded = expandAtomicOpToLibcall(
- I, Size, Align, I->getPointerOperand(), nullptr, nullptr,
+ I, Size, I->getAlign(), I->getPointerOperand(), nullptr, nullptr,
I->getOrdering(), AtomicOrdering::NotAtomic, Libcalls);
(void)expanded;
assert(expanded && "expandAtomicOpToLibcall shouldn't fail tor Load");
@@ -1547,11 +1514,10 @@ void AtomicExpand::expandAtomicStoreToLibcall(StoreInst *I) {
RTLIB::ATOMIC_STORE, RTLIB::ATOMIC_STORE_1, RTLIB::ATOMIC_STORE_2,
RTLIB::ATOMIC_STORE_4, RTLIB::ATOMIC_STORE_8, RTLIB::ATOMIC_STORE_16};
unsigned Size = getAtomicOpSize(I);
- unsigned Align = getAtomicOpAlign(I);
bool expanded = expandAtomicOpToLibcall(
- I, Size, Align, I->getPointerOperand(), I->getValueOperand(), nullptr,
- I->getOrdering(), AtomicOrdering::NotAtomic, Libcalls);
+ I, Size, I->getAlign(), I->getPointerOperand(), I->getValueOperand(),
+ nullptr, I->getOrdering(), AtomicOrdering::NotAtomic, Libcalls);
(void)expanded;
assert(expanded && "expandAtomicOpToLibcall shouldn't fail tor Store");
}
@@ -1562,10 +1528,9 @@ void AtomicExpand::expandAtomicCASToLibcall(AtomicCmpXchgInst *I) {
RTLIB::ATOMIC_COMPARE_EXCHANGE_2, RTLIB::ATOMIC_COMPARE_EXCHANGE_4,
RTLIB::ATOMIC_COMPARE_EXCHANGE_8, RTLIB::ATOMIC_COMPARE_EXCHANGE_16};
unsigned Size = getAtomicOpSize(I);
- unsigned Align = getAtomicOpAlign(I);
bool expanded = expandAtomicOpToLibcall(
- I, Size, Align, I->getPointerOperand(), I->getNewValOperand(),
+ I, Size, I->getAlign(), I->getPointerOperand(), I->getNewValOperand(),
I->getCompareOperand(), I->getSuccessOrdering(), I->getFailureOrdering(),
Libcalls);
(void)expanded;
@@ -1635,13 +1600,12 @@ void AtomicExpand::expandAtomicRMWToLibcall(AtomicRMWInst *I) {
ArrayRef<RTLIB::Libcall> Libcalls = GetRMWLibcall(I->getOperation());
unsigned Size = getAtomicOpSize(I);
- unsigned Align = getAtomicOpAlign(I);
bool Success = false;
if (!Libcalls.empty())
Success = expandAtomicOpToLibcall(
- I, Size, Align, I->getPointerOperand(), I->getValOperand(), nullptr,
- I->getOrdering(), AtomicOrdering::NotAtomic, Libcalls);
+ I, Size, I->getAlign(), I->getPointerOperand(), I->getValOperand(),
+ nullptr, I->getOrdering(), AtomicOrdering::NotAtomic, Libcalls);
// The expansion failed: either there were no libcalls at all for
// the operation (min/max), or there were only size-specialized
@@ -1672,7 +1636,7 @@ void AtomicExpand::expandAtomicRMWToLibcall(AtomicRMWInst *I) {
// 'I' are extracted from the Instruction subclass by the
// caller. Depending on the particular call, some will be null.
bool AtomicExpand::expandAtomicOpToLibcall(
- Instruction *I, unsigned Size, unsigned Align, Value *PointerOperand,
+ Instruction *I, unsigned Size, Align Alignment, Value *PointerOperand,
Value *ValueOperand, Value *CASExpected, AtomicOrdering Ordering,
AtomicOrdering Ordering2, ArrayRef<RTLIB::Libcall> Libcalls) {
assert(Libcalls.size() == 6);
@@ -1683,10 +1647,10 @@ bool AtomicExpand::expandAtomicOpToLibcall(
IRBuilder<> Builder(I);
IRBuilder<> AllocaBuilder(&I->getFunction()->getEntryBlock().front());
- bool UseSizedLibcall = canUseSizedAtomicCall(Size, Align, DL);
+ bool UseSizedLibcall = canUseSizedAtomicCall(Size, Alignment, DL);
Type *SizedIntTy = Type::getIntNTy(Ctx, Size * 8);
- const llvm::Align AllocaAlignment(DL.getPrefTypeAlignment(SizedIntTy));
+ const Align AllocaAlignment = DL.getPrefTypeAlign(SizedIntTy);
// TODO: the "order" argument type is "int", not int32. So
// getInt32Ty may be wrong if the arch uses e.g. 16-bit ints.
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 2087188e3edd..69a119bc3bf0 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -1542,6 +1542,13 @@ AtomicCmpXchgInst::AtomicCmpXchgInst(Value *Ptr, Value *Cmp, Value *NewVal,
Init(Ptr, Cmp, NewVal, SuccessOrdering, FailureOrdering, SSID);
}
+Align AtomicCmpXchgInst::getAlign() const {
+ // The default here is to assume it has NATURAL alignment, not
+ // DataLayout-specified alignment.
+ const DataLayout &DL = getModule()->getDataLayout();
+ return Align(DL.getTypeStoreSize(getCompareOperand()->getType()));
+}
+
//===----------------------------------------------------------------------===//
// AtomicRMWInst Implementation
//===----------------------------------------------------------------------===//
@@ -1623,6 +1630,13 @@ StringRef AtomicRMWInst::getOperationName(BinOp Op) {
llvm_unreachable("invalid atomicrmw operation");
}
+Align AtomicRMWInst::getAlign() const {
+ // The default here is to assume it has NATURAL alignment, not
+ // DataLayout-specified alignment.
+ const DataLayout &DL = getModule()->getDataLayout();
+ return Align(DL.getTypeStoreSize(getValOperand()->getType()));
+}
+
//===----------------------------------------------------------------------===//
// FenceInst Implementation
//===----------------------------------------------------------------------===//
More information about the llvm-commits
mailing list