diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h --- a/llvm/include/llvm/IR/Instructions.h +++ b/llvm/include/llvm/IR/Instructions.h @@ -590,6 +590,18 @@ /// 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(); @@ -597,8 +609,8 @@ /// 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(Ordering); } @@ -609,8 +621,8 @@ /// 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(Ordering); } diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp --- 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 @@ 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(Ptr->getType()) diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -5143,6 +5143,10 @@ ? 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 @@ 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 @@ 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 --- a/llvm/lib/IR/Instructions.cpp +++ b/llvm/lib/IR/Instructions.cpp @@ -1556,13 +1556,6 @@ "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 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -3826,29 +3826,7 @@ } 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(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::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(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 @@ &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 --- /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 --- /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 --- /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 --- /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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 GIT binary patch literal 0 Hc$@&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