From: JF Bastien Date: Thu, 17 Dec 2015 22:09:19 +0000 (+0000) Subject: Polish atomic pointers X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=7e54c38de03268a98f329069556b7bcf166c8f05 Polish atomic pointers Summary: I didn't realize that we already allowed atomic load/store of pointers, it was added in 2012 by r162146. This patch updates the documentation and tightens the verifier by using DataLayout to make sure that the stored size is byte-sized and power-of-two. DataLayout is also used for integers, and while I'm here I updated the corresponding code for cmpxchg and rmw. See the following discussion for context and upcoming changes to add floating-point and vector atomics: https://groups.google.com/forum/#!topic/llvm-dev/Nh0P_E3CRoo/discussion Reviewers: reames Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D15512 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@255931 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/docs/LangRef.rst b/docs/LangRef.rst index c968818acf2..103d876b3ce 100644 --- a/docs/LangRef.rst +++ b/docs/LangRef.rst @@ -6847,17 +6847,16 @@ then the optimizer is not allowed to modify the number or order of execution of this ``load`` with other :ref:`volatile operations `. -If the ``load`` is marked as ``atomic``, it takes an extra -:ref:`ordering ` and optional ``singlethread`` argument. The -``release`` and ``acq_rel`` orderings are not valid on ``load`` -instructions. Atomic loads produce :ref:`defined ` results -when they may see multiple atomic stores. The type of the pointee must -be an integer or floating point type whose bit width is a power of two, -greater than or equal to eight, and less than or equal to a -target-specific size limit. ``align`` must be explicitly specified on -atomic loads, and the load has undefined behavior if the alignment is -not set to a value which is at least the size in bytes of the pointee. -``!nontemporal`` does not have any defined semantics for atomic loads. +If the ``load`` is marked as ``atomic``, it takes an extra :ref:`ordering +` and optional ``singlethread`` argument. The ``release`` and +``acq_rel`` orderings are not valid on ``load`` instructions. Atomic loads +produce :ref:`defined ` results when they may see multiple atomic +stores. The type of the pointee must be an integer, pointer, or floating-point +type whose bit width is a power of two greater than or equal to eight and less +than or equal to a target-specific size limit. ``align`` must be explicitly +specified on atomic loads, and the load has undefined behavior if the alignment +is not set to a value which is at least the size in bytes of the +pointee. ``!nontemporal`` does not have any defined semantics for atomic loads. The optional constant ``align`` argument specifies the alignment of the operation (that is, the alignment of the memory address). A value of 0 @@ -6972,18 +6971,16 @@ then the optimizer is not allowed to modify the number or order of execution of this ``store`` with other :ref:`volatile operations `. -If the ``store`` is marked as ``atomic``, it takes an extra -:ref:`ordering ` and optional ``singlethread`` argument. The -``acquire`` and ``acq_rel`` orderings aren't valid on ``store`` -instructions. Atomic loads produce :ref:`defined ` results -when they may see multiple atomic stores. The type of the pointee must -be an integer or floating point type whose bit width is a power of two, -greater than or equal to eight, and less than or equal to a -target-specific size limit. ``align`` must be explicitly specified -on atomic stores, and the store has undefined behavior if the alignment -is not set to a value which is at least the size in bytes of the -pointee. ``!nontemporal`` does not have any defined semantics for -atomic stores. +If the ``store`` is marked as ``atomic``, it takes an extra :ref:`ordering +` and optional ``singlethread`` argument. The ``acquire`` and +``acq_rel`` orderings aren't valid on ``store`` instructions. Atomic loads +produce :ref:`defined ` results when they may see multiple atomic +stores. The type of the pointee must be an integer, pointer, or floating-point +type whose bit width is a power of two greater than or equal to eight and less +than or equal to a target-specific size limit. ``align`` must be explicitly +specified on atomic stores, and the store has undefined behavior if the +alignment is not set to a value which is at least the size in bytes of the +pointee. ``!nontemporal`` does not have any defined semantics for atomic stores. The optional constant ``align`` argument specifies the alignment of the operation (that is, the alignment of the memory address). A value of 0 diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index a70de2c7b93..1a22d37ba8a 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -207,6 +207,8 @@ class Verifier : public InstVisitor, VerifierSupport { /// Cache of constants visited in search of ConstantExprs. SmallPtrSet ConstantExprVisited; + void checkAtomicMemAccessSize(const Module *M, Type *Ty, + const Instruction *I); public: explicit Verifier(raw_ostream &OS) : VerifierSupport(OS), Context(nullptr), LandingPadResultTy(nullptr), @@ -2734,6 +2736,14 @@ void Verifier::visitRangeMetadata(Instruction& I, } } +void Verifier::checkAtomicMemAccessSize(const Module *M, Type *Ty, + const Instruction *I) { + unsigned Size = M->getDataLayout().getTypeSizeInBits(Ty); + Assert(Size >= 8, "atomic memory access' size must be byte-sized", Ty, I); + Assert(!(Size & (Size - 1)), + "atomic memory access' operand must have a power-of-two size", Ty, I); +} + void Verifier::visitLoadInst(LoadInst &LI) { PointerType *PTy = dyn_cast(LI.getOperand(0)->getType()); Assert(PTy, "Load operand must be a pointer.", &LI); @@ -2745,15 +2755,12 @@ void Verifier::visitLoadInst(LoadInst &LI) { "Load cannot have Release ordering", &LI); Assert(LI.getAlignment() != 0, "Atomic load must specify explicit alignment", &LI); - if (!ElTy->isPointerTy()) { - Assert(ElTy->isIntegerTy() || ElTy->isFloatingPointTy(), - "atomic load operand must have integer or floating point type!", - &LI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "atomic load operand must be power-of-two byte-sized integer", &LI, - ElTy); - } + Assert(ElTy->isIntegerTy() || ElTy->isPointerTy() || + ElTy->isFloatingPointTy(), + "atomic load operand must have integer, pointer, or floating point " + "type!", + ElTy, &LI); + checkAtomicMemAccessSize(M, ElTy, &LI); } else { Assert(LI.getSynchScope() == CrossThread, "Non-atomic load cannot have SynchronizationScope specified", &LI); @@ -2775,15 +2782,12 @@ void Verifier::visitStoreInst(StoreInst &SI) { "Store cannot have Acquire ordering", &SI); Assert(SI.getAlignment() != 0, "Atomic store must specify explicit alignment", &SI); - if (!ElTy->isPointerTy()) { - Assert(ElTy->isIntegerTy() || ElTy->isFloatingPointTy(), - "atomic store operand must have integer or floating point type!", - &SI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "atomic store operand must be power-of-two byte-sized integer", - &SI, ElTy); - } + Assert(ElTy->isIntegerTy() || ElTy->isPointerTy() || + ElTy->isFloatingPointTy(), + "atomic store operand must have integer, pointer, or floating point " + "type!", + ElTy, &SI); + checkAtomicMemAccessSize(M, ElTy, &SI); } else { Assert(SI.getSynchScope() == CrossThread, "Non-atomic store cannot have SynchronizationScope specified", &SI); @@ -2830,9 +2834,7 @@ void Verifier::visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI) { Type *ElTy = PTy->getElementType(); Assert(ElTy->isIntegerTy(), "cmpxchg operand must have integer type!", &CXI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "cmpxchg operand must be power-of-two byte-sized integer", &CXI, ElTy); + checkAtomicMemAccessSize(M, ElTy, &CXI); Assert(ElTy == CXI.getOperand(1)->getType(), "Expected value type does not match pointer operand type!", &CXI, ElTy); @@ -2851,10 +2853,7 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) { Type *ElTy = PTy->getElementType(); Assert(ElTy->isIntegerTy(), "atomicrmw operand must have integer type!", &RMWI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "atomicrmw operand must be power-of-two byte-sized integer", &RMWI, - ElTy); + checkAtomicMemAccessSize(M, ElTy, &RMWI); Assert(ElTy == RMWI.getOperand(1)->getType(), "Argument value type does not match pointer operand type!", &RMWI, ElTy); diff --git a/test/Verifier/atomics.ll b/test/Verifier/atomics.ll index ee7bf4bb19a..e49a0eb7beb 100644 --- a/test/Verifier/atomics.ll +++ b/test/Verifier/atomics.ll @@ -1,7 +1,7 @@ ; RUN: not opt -verify < %s 2>&1 | FileCheck %s -; CHECK: atomic store operand must have integer or floating point type! -; CHECK: atomic load operand must have integer or floating point type! +; CHECK: atomic store operand must have integer, pointer, or floating point type! +; CHECK: atomic load operand must have integer, pointer, or floating point type! define void @foo(x86_mmx* %P, x86_mmx %v) { store atomic x86_mmx %v, x86_mmx* %P unordered, align 8