[llvm] ffa15e9 - Extract isVolatile helper on Instruction [NFCI]
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 1 11:24:11 PDT 2021
Author: Philip Reames
Date: 2021-04-01T11:24:02-07:00
New Revision: ffa15e9463d07e9ba75e2c072afbc0c33f4f68a0
URL: https://github.com/llvm/llvm-project/commit/ffa15e9463d07e9ba75e2c072afbc0c33f4f68a0
DIFF: https://github.com/llvm/llvm-project/commit/ffa15e9463d07e9ba75e2c072afbc0c33f4f68a0.diff
LOG: Extract isVolatile helper on Instruction [NFCI]
We have this logic duplicated in several cases, none of which were exhaustive. Consolidate it in one place.
I don't believe this actually impacts behavior of the callers. I think they all filter their inputs such that their partial implementations were correct. If not, this might be fixing a cornercase bug.
Added:
Modified:
llvm/include/llvm/IR/Instruction.h
llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
llvm/lib/IR/Instruction.cpp
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index b99dc62bbb9d..bf73d4155b8a 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -597,6 +597,9 @@ class Instruction : public User,
/// Return true if this atomic instruction stores to memory.
bool hasAtomicStore() const;
+ /// Return true if this instruction has a volatile memory access.
+ bool isVolatile() const;
+
/// Return true if this instruction may throw an exception.
bool mayThrow() const;
diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 3131da2f8b0a..01321e7eb2c1 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -243,16 +243,6 @@ MemDepResult MemoryDependenceResults::getCallDependencyFrom(
return MemDepResult::getNonFuncLocal();
}
-static bool isVolatile(Instruction *Inst) {
- if (auto *LI = dyn_cast<LoadInst>(Inst))
- return LI->isVolatile();
- if (auto *SI = dyn_cast<StoreInst>(Inst))
- return SI->isVolatile();
- if (auto *AI = dyn_cast<AtomicCmpXchgInst>(Inst))
- return AI->isVolatile();
- return false;
-}
-
MemDepResult MemoryDependenceResults::getPointerDependencyFrom(
const MemoryLocation &MemLoc, bool isLoad, BasicBlock::iterator ScanIt,
BasicBlock *BB, Instruction *QueryInst, unsigned *Limit) {
@@ -491,7 +481,7 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
if (!QueryInst)
// Original QueryInst *may* be volatile
return MemDepResult::getClobber(LI);
- if (isVolatile(QueryInst))
+ if (QueryInst->isVolatile())
// Ordering required if QueryInst is itself volatile
return MemDepResult::getClobber(LI);
// Otherwise, volatile doesn't imply any special ordering
@@ -882,7 +872,7 @@ void MemoryDependenceResults::getNonLocalPointerDependency(
}
return false;
};
- if (isVolatile(QueryInst) || isOrdered(QueryInst)) {
+ if (QueryInst->isVolatile() || isOrdered(QueryInst)) {
Result.push_back(NonLocalDepResult(FromBB, MemDepResult::getUnknown(),
const_cast<Value *>(Loc.Ptr)));
return;
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 8e52dd3ddc71..87a69a8ed232 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -618,6 +618,36 @@ bool Instruction::hasAtomicStore() const {
}
}
+bool Instruction::isVolatile() const {
+ switch (getOpcode()) {
+ default:
+ return false;
+ case Instruction::AtomicRMW:
+ return cast<AtomicRMWInst>(this)->isVolatile();
+ case Instruction::Store:
+ return cast<StoreInst>(this)->isVolatile();
+ case Instruction::Load:
+ return cast<LoadInst>(this)->isVolatile();
+ case Instruction::AtomicCmpXchg:
+ return cast<AtomicCmpXchgInst>(this)->isVolatile();
+ case Instruction::Call:
+ case Instruction::Invoke:
+ // There are a very limited number of intrinsics with volatile flags.
+ if (auto *II = dyn_cast<IntrinsicInst>(this)) {
+ if (auto *MI = dyn_cast<MemIntrinsic>(II))
+ return MI->isVolatile();
+ switch (II->getIntrinsicID()) {
+ default: break;
+ case Intrinsic::matrix_column_major_load:
+ return cast<ConstantInt>(II->getArgOperand(2))->isOne();
+ case Intrinsic::matrix_column_major_store:
+ return cast<ConstantInt>(II->getArgOperand(3))->isOne();
+ }
+ }
+ return false;
+ }
+}
+
bool Instruction::mayThrow() const {
if (const CallInst *CI = dyn_cast<CallInst>(this))
return !CI->doesNotThrow();
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 71d0146432cc..a82970fafb1d 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -155,27 +155,22 @@ getAssumedConstantInt(Attributor &A, const Value &V,
/// is set to false and the instruction is volatile, return nullptr.
static const Value *getPointerOperand(const Instruction *I,
bool AllowVolatile) {
+ if (!AllowVolatile && I->isVolatile())
+ return nullptr;
+
if (auto *LI = dyn_cast<LoadInst>(I)) {
- if (!AllowVolatile && LI->isVolatile())
- return nullptr;
return LI->getPointerOperand();
}
if (auto *SI = dyn_cast<StoreInst>(I)) {
- if (!AllowVolatile && SI->isVolatile())
- return nullptr;
return SI->getPointerOperand();
}
if (auto *CXI = dyn_cast<AtomicCmpXchgInst>(I)) {
- if (!AllowVolatile && CXI->isVolatile())
- return nullptr;
return CXI->getPointerOperand();
}
if (auto *RMWI = dyn_cast<AtomicRMWInst>(I)) {
- if (!AllowVolatile && RMWI->isVolatile())
- return nullptr;
return RMWI->getPointerOperand();
}
@@ -1287,9 +1282,6 @@ struct AANoSyncImpl : AANoSync {
/// or monotonic ordering
static bool isNonRelaxedAtomic(Instruction *I);
- /// Helper function used to determine whether an instruction is volatile.
- static bool isVolatile(Instruction *I);
-
/// Helper function uset to check if intrinsic is volatile (memcpy, memmove,
/// memset).
static bool isNoSyncIntrinsic(Instruction *I);
@@ -1360,23 +1352,6 @@ bool AANoSyncImpl::isNoSyncIntrinsic(Instruction *I) {
return false;
}
-bool AANoSyncImpl::isVolatile(Instruction *I) {
- assert(!isa<CallBase>(I) && "Calls should not be checked here");
-
- switch (I->getOpcode()) {
- case Instruction::AtomicRMW:
- return cast<AtomicRMWInst>(I)->isVolatile();
- case Instruction::Store:
- return cast<StoreInst>(I)->isVolatile();
- case Instruction::Load:
- return cast<LoadInst>(I)->isVolatile();
- case Instruction::AtomicCmpXchg:
- return cast<AtomicCmpXchgInst>(I)->isVolatile();
- default:
- return false;
- }
-}
-
ChangeStatus AANoSyncImpl::updateImpl(Attributor &A) {
auto CheckRWInstForNoSync = [&](Instruction &I) {
@@ -1395,7 +1370,7 @@ ChangeStatus AANoSyncImpl::updateImpl(Attributor &A) {
return NoSyncAA.isAssumedNoSync();
}
- if (!isVolatile(&I) && !isNonRelaxedAtomic(&I))
+ if (!I.isVolatile() && !isNonRelaxedAtomic(&I))
return true;
return false;
More information about the llvm-commits
mailing list