[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

llvm/include/llvm/IR/Instructions.h

@@ -590,6 +590,18 @@ public:
   /// 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 @@ public:
 
   /// 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 @@ public:
 
   /// 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);
   }
 

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())

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];
 

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,

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);

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
+}

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
+}

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
+}

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
+}

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