[llvm] 7a29a12 - [Verifier] Move some atomicrmw/cmpxchg checks to instruction creation
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Fri May 21 13:42:59 PDT 2021
Author: Arthur Eubanks
Date: 2021-05-21T13:41:17-07:00
New Revision: 7a29a1230148385e93493891cc7eb7f7f3b4a6cb
URL: https://github.com/llvm/llvm-project/commit/7a29a1230148385e93493891cc7eb7f7f3b4a6cb
DIFF: https://github.com/llvm/llvm-project/commit/7a29a1230148385e93493891cc7eb7f7f3b4a6cb.diff
LOG: [Verifier] Move some atomicrmw/cmpxchg checks to instruction creation
These checks already exist as asserts when creating the corresponding
instruction. Anybody creating these instructions already need to take
care to not break these checks.
Move the checks for success/failure ordering in cmpxchg from the
verifier to the LLParser and BitcodeReader plus an assert.
Add some tests for cmpxchg ordering. The .bc files are created from the
.ll files with an llvm-as with these checks disabled.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D102803
Added:
llvm/test/Assembler/cmpxchg-ordering-2.ll
llvm/test/Assembler/cmpxchg-ordering-3.ll
llvm/test/Assembler/cmpxchg-ordering-4.ll
llvm/test/Assembler/cmpxchg-ordering.ll
llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-2.bc
llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-3.bc
llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-4.bc
llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering.bc
Modified:
llvm/include/llvm/IR/Instructions.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/Verifier.cpp
llvm/test/Bitcode/invalid.test
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index 47693f40f6432..a94f5406f3b25 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -590,6 +590,18 @@ class AtomicCmpXchgInst : public Instruction {
/// Transparently provide more efficient getOperand methods.
DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value);
+ static bool isValidSuccessOrdering(AtomicOrdering Ordering) {
+ return Ordering != AtomicOrdering::NotAtomic &&
+ Ordering != AtomicOrdering::Unordered;
+ }
+
+ static bool isValidFailureOrdering(AtomicOrdering Ordering) {
+ return Ordering != AtomicOrdering::NotAtomic &&
+ Ordering != AtomicOrdering::Unordered &&
+ Ordering != AtomicOrdering::AcquireRelease &&
+ Ordering != AtomicOrdering::Release;
+ }
+
/// Returns the success ordering constraint of this cmpxchg instruction.
AtomicOrdering getSuccessOrdering() const {
return getSubclassData<SuccessOrderingField>();
@@ -597,8 +609,8 @@ class AtomicCmpXchgInst : public Instruction {
/// Sets the success ordering constraint of this cmpxchg instruction.
void setSuccessOrdering(AtomicOrdering Ordering) {
- assert(Ordering != AtomicOrdering::NotAtomic &&
- "CmpXchg instructions can only be atomic.");
+ assert(isValidSuccessOrdering(Ordering) &&
+ "invalid CmpXchg success ordering");
setSubclassData<SuccessOrderingField>(Ordering);
}
@@ -609,8 +621,8 @@ class AtomicCmpXchgInst : public Instruction {
/// Sets the failure ordering constraint of this cmpxchg instruction.
void setFailureOrdering(AtomicOrdering Ordering) {
- assert(Ordering != AtomicOrdering::NotAtomic &&
- "CmpXchg instructions can only be atomic.");
+ assert(isValidFailureOrdering(Ordering) &&
+ "invalid CmpXchg failure ordering");
setSubclassData<FailureOrderingField>(Ordering);
}
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 88fd26fdb7d5c..3184aba838ac9 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -32,6 +32,7 @@
#include "llvm/IR/GlobalIFunc.h"
#include "llvm/IR/GlobalObject.h"
#include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/Instructions.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Metadata.h"
@@ -7582,13 +7583,10 @@ int LLParser::parseCmpXchg(Instruction *&Inst, PerFunctionState &PFS) {
parseOptionalCommaAlign(Alignment, AteExtraComma))
return true;
- if (SuccessOrdering == AtomicOrdering::Unordered ||
- FailureOrdering == AtomicOrdering::Unordered)
- return tokError("cmpxchg cannot be unordered");
- if (FailureOrdering == AtomicOrdering::Release ||
- FailureOrdering == AtomicOrdering::AcquireRelease)
- return tokError(
- "cmpxchg failure ordering cannot include release semantics");
+ if (!AtomicCmpXchgInst::isValidSuccessOrdering(SuccessOrdering))
+ return tokError("invalid cmpxchg success ordering");
+ if (!AtomicCmpXchgInst::isValidFailureOrdering(FailureOrdering))
+ return tokError("invalid cmpxchg failure ordering");
if (!Ptr->getType()->isPointerTy())
return error(PtrLoc, "cmpxchg operand must be a pointer");
if (!cast<PointerType>(Ptr->getType())
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index ea655f99de955..7d23f3b5728f9 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -5143,6 +5143,10 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
? AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrdering)
: getDecodedOrdering(Record[OpNum + 3]);
+ if (FailureOrdering == AtomicOrdering::NotAtomic ||
+ FailureOrdering == AtomicOrdering::Unordered)
+ return error("Invalid record");
+
const Align Alignment(
TheModule->getDataLayout().getTypeStoreSize(Cmp->getType()));
@@ -5192,9 +5196,8 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
const AtomicOrdering SuccessOrdering =
getDecodedOrdering(Record[OpNum + 1]);
- if (SuccessOrdering == AtomicOrdering::NotAtomic ||
- SuccessOrdering == AtomicOrdering::Unordered)
- return error("Invalid record");
+ if (!AtomicCmpXchgInst::isValidSuccessOrdering(SuccessOrdering))
+ return error("Invalid cmpxchg success ordering");
const SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 2]);
@@ -5203,6 +5206,8 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
const AtomicOrdering FailureOrdering =
getDecodedOrdering(Record[OpNum + 3]);
+ if (!AtomicCmpXchgInst::isValidFailureOrdering(FailureOrdering))
+ return error("Invalid cmpxchg failure ordering");
const bool IsWeak = Record[OpNum + 4];
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 2415b9b244431..f1df5001c134b 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -1556,13 +1556,6 @@ void AtomicCmpXchgInst::Init(Value *Ptr, Value *Cmp, Value *NewVal,
"Ptr must be a pointer to NewVal type!");
assert(getOperand(1)->getType() == getOperand(2)->getType() &&
"Cmp type and NewVal type must be same!");
- assert(SuccessOrdering != AtomicOrdering::NotAtomic &&
- "AtomicCmpXchg instructions must be atomic!");
- assert(FailureOrdering != AtomicOrdering::NotAtomic &&
- "AtomicCmpXchg instructions must be atomic!");
- assert(FailureOrdering != AtomicOrdering::Release &&
- FailureOrdering != AtomicOrdering::AcquireRelease &&
- "AtomicCmpXchg failure ordering cannot include release semantics");
}
AtomicCmpXchgInst::AtomicCmpXchgInst(Value *Ptr, Value *Cmp, Value *NewVal,
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index fb5c1a3088b92..80f343365afda 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -3826,29 +3826,7 @@ void Verifier::visitAllocaInst(AllocaInst &AI) {
}
void Verifier::visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI) {
-
- // FIXME: more conditions???
- Assert(CXI.getSuccessOrdering() != AtomicOrdering::NotAtomic,
- "cmpxchg instructions must be atomic.", &CXI);
- Assert(CXI.getFailureOrdering() != AtomicOrdering::NotAtomic,
- "cmpxchg instructions must be atomic.", &CXI);
- Assert(CXI.getSuccessOrdering() != AtomicOrdering::Unordered,
- "cmpxchg instructions cannot be unordered.", &CXI);
- Assert(CXI.getFailureOrdering() != AtomicOrdering::Unordered,
- "cmpxchg instructions cannot be unordered.", &CXI);
- Assert(CXI.getFailureOrdering() != AtomicOrdering::Release &&
- CXI.getFailureOrdering() != AtomicOrdering::AcquireRelease,
- "cmpxchg failure ordering cannot include release semantics", &CXI);
-
- PointerType *PTy = dyn_cast<PointerType>(CXI.getOperand(0)->getType());
- Assert(PTy, "First cmpxchg operand must be a pointer.", &CXI);
Type *ElTy = CXI.getOperand(1)->getType();
- Assert(PTy->isOpaqueOrPointeeTypeMatches(ElTy),
- "Expected value type does not match pointer operand type!", &CXI,
- ElTy);
- Assert(ElTy == CXI.getOperand(2)->getType(),
- "Stored value type does not match expected value operand type!", &CXI,
- ElTy);
Assert(ElTy->isIntOrPtrTy(),
"cmpxchg operand must have integer or pointer type", ElTy, &CXI);
checkAtomicMemAccessSize(ElTy, &CXI);
@@ -3856,13 +3834,9 @@ void Verifier::visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI) {
}
void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
- Assert(RMWI.getOrdering() != AtomicOrdering::NotAtomic,
- "atomicrmw instructions must be atomic.", &RMWI);
Assert(RMWI.getOrdering() != AtomicOrdering::Unordered,
"atomicrmw instructions cannot be unordered.", &RMWI);
auto Op = RMWI.getOperation();
- PointerType *PTy = dyn_cast<PointerType>(RMWI.getOperand(0)->getType());
- Assert(PTy, "First atomicrmw operand must be a pointer.", &RMWI);
Type *ElTy = RMWI.getOperand(1)->getType();
if (Op == AtomicRMWInst::Xchg) {
Assert(ElTy->isIntegerTy() || ElTy->isFloatingPointTy(), "atomicrmw " +
@@ -3881,9 +3855,6 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
&RMWI, ElTy);
}
checkAtomicMemAccessSize(ElTy, &RMWI);
- Assert(PTy->isOpaqueOrPointeeTypeMatches(ElTy),
- "Argument value type does not match pointer operand type!", &RMWI,
- ElTy);
Assert(AtomicRMWInst::FIRST_BINOP <= Op && Op <= AtomicRMWInst::LAST_BINOP,
"Invalid binary operation!", &RMWI);
visitInstruction(RMWI);
diff --git a/llvm/test/Assembler/cmpxchg-ordering-2.ll b/llvm/test/Assembler/cmpxchg-ordering-2.ll
new file mode 100644
index 0000000000000..5bbb7e8b10368
--- /dev/null
+++ b/llvm/test/Assembler/cmpxchg-ordering-2.ll
@@ -0,0 +1,7 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+define void @f(i32* %a, i32 %b, i32 %c) {
+; CHECK: invalid cmpxchg failure ordering
+ %x = cmpxchg i32* %a, i32 %b, i32 %c acq_rel unordered
+ ret void
+}
diff --git a/llvm/test/Assembler/cmpxchg-ordering-3.ll b/llvm/test/Assembler/cmpxchg-ordering-3.ll
new file mode 100644
index 0000000000000..fa760b217c11a
--- /dev/null
+++ b/llvm/test/Assembler/cmpxchg-ordering-3.ll
@@ -0,0 +1,7 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+define void @f(i32* %a, i32 %b, i32 %c) {
+; CHECK: invalid cmpxchg failure ordering
+ %x = cmpxchg i32* %a, i32 %b, i32 %c acq_rel acq_rel
+ ret void
+}
diff --git a/llvm/test/Assembler/cmpxchg-ordering-4.ll b/llvm/test/Assembler/cmpxchg-ordering-4.ll
new file mode 100644
index 0000000000000..d33100daf0dc5
--- /dev/null
+++ b/llvm/test/Assembler/cmpxchg-ordering-4.ll
@@ -0,0 +1,7 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+define void @f(i32* %a, i32 %b, i32 %c) {
+; CHECK: invalid cmpxchg failure ordering
+ %x = cmpxchg i32* %a, i32 %b, i32 %c acq_rel release
+ ret void
+}
diff --git a/llvm/test/Assembler/cmpxchg-ordering.ll b/llvm/test/Assembler/cmpxchg-ordering.ll
new file mode 100644
index 0000000000000..62462ce3da134
--- /dev/null
+++ b/llvm/test/Assembler/cmpxchg-ordering.ll
@@ -0,0 +1,7 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+define void @f(i32* %a, i32 %b, i32 %c) {
+; CHECK: invalid cmpxchg success ordering
+ %x = cmpxchg i32* %a, i32 %b, i32 %c unordered monotonic
+ ret void
+}
diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-2.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-2.bc
new file mode 100644
index 0000000000000..76693f78c5aa7
Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-2.bc
diff er
diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-3.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-3.bc
new file mode 100644
index 0000000000000..2421e5bb47750
Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-3.bc
diff er
diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-4.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-4.bc
new file mode 100644
index 0000000000000..711a0a350be7b
Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-4.bc
diff er
diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering.bc
new file mode 100644
index 0000000000000..c319f46f536dd
Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering.bc
diff er
diff --git a/llvm/test/Bitcode/invalid.test b/llvm/test/Bitcode/invalid.test
index 90303586ec3b9..26bea8c2c7b1a 100644
--- a/llvm/test/Bitcode/invalid.test
+++ b/llvm/test/Bitcode/invalid.test
@@ -235,3 +235,14 @@ RUN: not llvm-dis -disable-output %p/Inputs/invalid-fcmp-opnum.bc 2>&1 | \
RUN: FileCheck --check-prefix=INVALID-FCMP-OPNUM %s
INVALID-FCMP-OPNUM: Invalid record: operand number exceeded available operands
+
+RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering.bc 2>&1 | \
+RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s
+RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering-2.bc 2>&1 | \
+RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s
+RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering-3.bc 2>&1 | \
+RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s
+RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering-4.bc 2>&1 | \
+RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s
+
+CMPXCHG-ORDERING: Invalid cmpxchg {{failure|success}} ordering
More information about the llvm-commits
mailing list