Bring AMDGPU memory model related bits in sync with phabricator reviews

docs/LangRef.rst

@@ -2164,18 +2164,19 @@ If an atomic operation is marked ``singlethread``, it only *synchronizes with*,
 and only participates in the seq\_cst total orderings of, other operations
 running in the same thread (for example, in signal handlers).
 
-.. _synchscope:
+.. _syncscope:
 
-If an atomic operation is marked ``syncscope(<n>)``, then it is target defined
-how ``<n>`` affects which other operations it *synchronizes with*, and with
-which other operations it participates in the seq\_cst total orderings (for
-example, OpenCL supports separate memory scopes for device, work-group and
-sub-group).
+If an atomic operation is marked ``syncscope(<n>)``, then it
+*synchronizes with*, and participates in the seq\_cst total orderings of, other
+atomic operations marked ``syncscope(<n>)``. It is target defined how it
+interacts with atomic operations marked ``singlethread``, marked
+``syncscope(<m>)`` where ``m != n``, or not marked ``singlethread`` or
+``syncscope(<n>)``.
 
 Otherwise, an atomic operation that is not marked ``singlethread`` or
 ``syncscope(<n>)`` *synchronizes with*, and participates in the global seq\_cst
-total orderings of, other operations that do not specify ``singlethread`` or
-``syncscope(<n>)`` running in any thread.
+total orderings of, other operations that are not marked ``singlethread`` or
+``syncscope(<n>)``.
 
 .. _fastmath:
 
@@ -7032,7 +7033,7 @@ Syntax:
 ::
 
       <result> = load [volatile] <ty>, <ty>* <pointer>[, align <alignment>][, !nontemporal !<index>][, !invariant.load !<index>][, !invariant.group !<index>][, !nonnull !<index>][, !dereferenceable !<deref_bytes_node>][, !dereferenceable_or_null !<deref_bytes_node>][, !align !<align_node>]
-      <result> = load atomic [volatile] <ty>, <ty>* <pointer> [singlethread|synchscope(<n>)] <ordering>, align <alignment> [, !invariant.group !<index>]
+      <result> = load atomic [volatile] <ty>, <ty>* <pointer> [singlethread|syncscope(<n>)] <ordering>, align <alignment> [, !invariant.group !<index>]
       !<index> = !{ i32 1 }
       !<deref_bytes_node> = !{i64 <dereferenceable_bytes>}
       !<align_node> = !{ i64 <value_alignment> }
@@ -7053,7 +7054,7 @@ modify the number or order of execution of this ``load`` with other
 :ref:`volatile operations <volatile>`.
 
 If the ``load`` is marked as ``atomic``, it takes an extra :ref:`ordering
-<ordering>` and optional ``singlethread`` or ``synchscope(<n>)`` argument. The
+<ordering>` and optional ``singlethread`` or ``syncscope(<n>)`` argument. The
 ``release`` and ``acq_rel`` orderings are not valid on ``load`` instructions.
 Atomic loads produce :ref:`defined <memmodel>` results when they may see
 multiple atomic stores. The type of the pointee must be an integer, pointer, or
@@ -7161,7 +7162,7 @@ Syntax:
 ::
 
       store [volatile] <ty> <value>, <ty>* <pointer>[, align <alignment>][, !nontemporal !<index>][, !invariant.group !<index>]        ; yields void
-      store atomic [volatile] <ty> <value>, <ty>* <pointer> [singlethread|synchscope(<n>)] <ordering>, align <alignment> [, !invariant.group !<index>] ; yields void
+      store atomic [volatile] <ty> <value>, <ty>* <pointer> [singlethread|syncscope(<n>)] <ordering>, align <alignment> [, !invariant.group !<index>] ; yields void
 
 Overview:
 """""""""
@@ -7181,7 +7182,7 @@ allowed to modify the number or order of execution of this ``store`` with other
 structural type <t_opaque>`) can be stored.
 
 If the ``store`` is marked as ``atomic``, it takes an extra :ref:`ordering
-<ordering>` and optional ``singlethread`` or ``synchscope(<n>)`` argument. The
+<ordering>` and optional ``singlethread`` or ``syncscope(<n>)`` argument. The
 ``acquire`` and ``acq_rel`` orderings aren't valid on ``store`` instructions.
 Atomic loads produce :ref:`defined <memmodel>` results when they may see
 multiple atomic stores. The type of the pointee must be an integer, pointer, or
@@ -7249,7 +7250,7 @@ Syntax:
 
 ::
 
-      fence [singlethread|synchscope(<n>)] <ordering>            ; yields void
+      fence [singlethread|syncscope(<n>)] <ordering>            ; yields void
 
 Overview:
 """""""""
@@ -7283,12 +7284,8 @@ A ``fence`` which has ``seq_cst`` ordering, in addition to having both
 ``acquire`` and ``release`` semantics specified above, participates in
 the global program order of other ``seq_cst`` operations and/or fences.
 
-The optional ":ref:`singlethread <singlethread>`" argument specifies
-that the fence only synchronizes with other fences in the same thread.
-(This is useful for interacting with signal handlers.)
-
-If ``fence`` is marked ``syncscope(<n>)``, then it is target defined how ``<n>``
-affects which other operations it *synchronizes with*.
+A ``fence`` instruction can also take an optional
+":ref:`singlethread <singlethread>`" or ":ref:`syncscope <syncscope>`" argument.
 
 Example:
 """"""""
@@ -7297,7 +7294,7 @@ Example:
 
       fence acquire                          ; yields void
       fence singlethread seq_cst             ; yields void
-      fence synchscope(2) seq_cst            ; yields void
+      fence syncscope(2) seq_cst             ; yields void
 
 .. _i_cmpxchg:
 
@@ -7309,7 +7306,7 @@ Syntax:
 
 ::
 
-      cmpxchg [weak] [volatile] <ty>* <pointer>, <ty> <cmp>, <ty> <new> [singlethread|synchscope(<n>)] <success ordering> <failure ordering> ; yields  { ty, i1 }
+      cmpxchg [weak] [volatile] <ty>* <pointer>, <ty> <cmp>, <ty> <new> [singlethread|syncscope(<n>)] <success ordering> <failure ordering> ; yields  { ty, i1 }
 
 Overview:
 """""""""
@@ -7338,12 +7335,8 @@ must be at least ``monotonic``, the ordering constraint on failure must be no
 stronger than that on success, and the failure ordering cannot be either
 ``release`` or ``acq_rel``.
 
-The optional "``singlethread``" argument declares that the ``cmpxchg``
-is only atomic with respect to code (usually signal handlers) running in
-the same thread as the ``cmpxchg``.
-
-If ``cmpxchg`` is marked ``syncscope(<n>)``, then it is target defined how
-``<n>`` affects which other operations it *synchronizes with*.
+A ``cmpxchg`` instruction can also take an optional
+":ref:`singlethread <singlethread>`" or ":ref:`syncscope <syncscope>`" argument.
 
 The pointer passed into cmpxchg must have alignment greater than or
 equal to the size in memory of the operand.
@@ -7397,7 +7390,7 @@ Syntax:
 
 ::
 
-      atomicrmw [volatile] <operation> <ty>* <pointer>, <ty> <value> [singlethread|synchscope(<n>)] <ordering>                   ; yields ty
+      atomicrmw [volatile] <operation> <ty>* <pointer>, <ty> <value> [singlethread|syncscope(<n>)] <ordering>                   ; yields ty
 
 Overview:
 """""""""
@@ -7431,12 +7424,8 @@ be a pointer to that type. If the ``atomicrmw`` is marked as
 order of execution of this ``atomicrmw`` with other :ref:`volatile
 operations <volatile>`.
 
-The optional "``singlethread``" argument declares that the ``atomicrmw``
-is only atomic with respect to code (usually signal handlers) running in
-the same thread as the ``atomicrmw``.
-
-If ``atomicrmw`` is marked ``syncscope(<n>)``, then it is target defined how
-``<n>`` affects which other operations it *synchronizes with*.
+A ``atomicrmw`` instruction can also take an optional
+":ref:`singlethread <singlethread>`" or ":ref:`syncscope <syncscope>`" argument.
 
 Semantics:
 """"""""""

include/llvm/Bitcode/LLVMBitCodes.h

@@ -370,7 +370,7 @@ enum AtomicOrderingCodes {
 };
 
 /// Encoded SynchronizationScope values.
-enum AtomicSynchScopeCodes : unsigned {
+enum AtomicSynchScopeCodes : uint8_t {
   /// Encoded value for SingleThread synchronization scope.
   SYNCHSCOPE_SINGLETHREAD = 0,
 

include/llvm/CodeGen/MachineMemOperand.h

@@ -117,15 +117,38 @@ class MachineMemOperand {
   };
 
 private:
+  /// Atomic information for this memory operation.
+  struct MachineAtomicInfo {
+    /// Synchronization scope for this memory operation.
+    unsigned SynchScope : 8;      // enum SynchronizationScope
+    /// Atomic ordering requirements for this memory operation. For cmpxchg
+    /// atomic operations, atomic ordering requirements when store occurs.
+    unsigned Ordering : 4;        // enum AtomicOrdering
+    /// For cmpxchg atomic operations, atomic ordering requirements when store
+    /// does not occur.
+    unsigned FailureOrdering : 4; // enum AtomicOrdering
+    /// Reserved/unused.
+    unsigned Reserved : 16;
+  };
+
   MachinePointerInfo PtrInfo;
   uint64_t Size;
   Flags FlagVals;
   uint16_t BaseAlignLog2; // log_2(base_alignment) + 1
+  MachineAtomicInfo AtomicInfo;
   AAMDNodes AAInfo;
   const MDNode *Ranges;
-  SynchronizationScope SynchScope;
-  AtomicOrdering Ordering;
-  AtomicOrdering FailureOrdering;
+
+  /// Initialize atomic information for this memory operation.
+  void InitAtomicInfo(SynchronizationScope SynchScope, AtomicOrdering Ordering,
+                      AtomicOrdering FailureOrdering) {
+    AtomicInfo.SynchScope = static_cast<unsigned>(SynchScope);
+    assert(getSynchScope() == SynchScope && "Value truncated");
+    AtomicInfo.Ordering = static_cast<unsigned>(Ordering);
+    assert(getOrdering() == Ordering && "Value truncated");
+    AtomicInfo.FailureOrdering = static_cast<unsigned>(FailureOrdering);
+    assert(getFailureOrdering() == FailureOrdering && "Value truncated");
+  }
 
 public:
   /// Construct a MachineMemOperand object with the specified PtrInfo, flags,
@@ -188,18 +211,26 @@ class MachineMemOperand {
   const MDNode *getRanges() const { return Ranges; }
 
   /// Return the synchronization scope for this memory operation.
-  SynchronizationScope getSynchScope() const { return SynchScope; }
+  SynchronizationScope getSynchScope() const {
+    return static_cast<SynchronizationScope>(AtomicInfo.SynchScope);
+  }
 
   /// Return the atomic ordering requirements for this memory operation.
-  AtomicOrdering getOrdering() const { return Ordering; }
+  AtomicOrdering getOrdering() const {
+    return static_cast<AtomicOrdering>(AtomicInfo.Ordering);
+  }
 
   /// For cmpxchg atomic operations, return the atomic ordering requirements
   /// when store occurs.
-  AtomicOrdering getSuccessOrdering() const { return getOrdering(); }
+  AtomicOrdering getSuccessOrdering() const {
+    return getOrdering();
+  }
 
   /// For cmpxchg atomic operations, return the atomic ordering requirements
   /// when store does not occur.
-  AtomicOrdering getFailureOrdering() const { return FailureOrdering; }
+  AtomicOrdering getFailureOrdering() const {
+    return static_cast<AtomicOrdering>(AtomicInfo.FailureOrdering);
+  }
 
   bool isLoad() const { return FlagVals & MOLoad; }
   bool isStore() const { return FlagVals & MOStore; }

include/llvm/IR/Instructions.h

@@ -38,7 +38,7 @@ class DataLayout;
 class LLVMContext;
 
 /// Predefined synchronization scopes.
-enum SynchronizationScope : unsigned {
+enum SynchronizationScope : uint8_t {
   /// Synchronized with respect to signal handlers executing in the same thread.
   SingleThread = 0,
 

lib/AsmParser/LLLexer.cpp

@@ -541,7 +541,7 @@ lltok::Kind LLLexer::LexIdentifier() {
   KEYWORD(acq_rel);
   KEYWORD(seq_cst);
   KEYWORD(singlethread);
-  KEYWORD(synchscope);
+  KEYWORD(syncscope);
 
   KEYWORD(nnan);
   KEYWORD(ninf);

lib/AsmParser/LLParser.cpp

@@ -1274,6 +1274,19 @@ bool LLParser::ParseStringConstant(std::string &Result) {
   return false;
 }
 
+/// ParseUInt8
+///   ::= uint8
+bool LLParser::ParseUInt8(uint8_t &Val) {
+  if (Lex.getKind() != lltok::APSInt || Lex.getAPSIntVal().isSigned())
+    return TokError("expected integer");
+  uint64_t Val64 = Lex.getAPSIntVal().getLimitedValue(0xFFULL+1);
+  if (Val64 != uint8_t(Val64))
+    return TokError("expected 8-bit integer (too large)");
+  Val = Val64;
+  Lex.Lex();
+  return false;
+}
+
 /// ParseUInt32
 ///   ::= uint32
 bool LLParser::ParseUInt32(uint32_t &Val) {
@@ -1888,7 +1901,7 @@ bool LLParser::parseAllocSizeArguments(unsigned &BaseSizeArg,
 
 /// ParseScopeAndOrdering
 ///   if isAtomic:
-///     ::= 'singlethread' or 'synchscope' '(' uint32 ')'? AtomicOrdering
+///     ::= 'singlethread' or 'syncscope' '(' uint8 ')'? AtomicOrdering
 ///   else
 ///     ::=
 ///
@@ -1904,27 +1917,27 @@ bool LLParser::ParseScopeAndOrdering(bool isAtomic, SynchronizationScope &Scope,
 /// ParseScope
 ///   ::= /* empty */
 ///   ::= 'singlethread'
-///   ::= 'synchscope' '(' uint32 ')'
+///   ::= 'syncscope' '(' uint8 ')'
 ///
 /// This sets Scope to the parsed value.
 bool LLParser::ParseScope(SynchronizationScope &Scope) {
-  if (EatIfPresent(lltok::kw_synchscope)) {
+  if (EatIfPresent(lltok::kw_syncscope)) {
     auto StartParen = Lex.getLoc();
     if (!EatIfPresent(lltok::lparen))
-      return Error(StartParen, "expected '(' in synchscope");
+      return Error(StartParen, "expected '(' in syncscope");
 
-    unsigned ScopeU32 = 0;
-    auto ScopeU32At = Lex.getLoc();
-    if (ParseUInt32(ScopeU32))
+    uint8_t ScopeU8 = 0;
+    auto ScopeU8At = Lex.getLoc();
+    if (ParseUInt8(ScopeU8))
       return true;
-    if (ScopeU32 < SynchronizationScopeFirstTargetSpecific)
-      return Error(ScopeU32At, "invalid target specific synchronization scope");
+    if (ScopeU8 < SynchronizationScopeFirstTargetSpecific)
+      return Error(ScopeU8At, "invalid syncscope");
 
     auto EndParen = Lex.getLoc();
     if (!EatIfPresent(lltok::rparen))
-      return Error(EndParen, "expected ')' in synchscope");
+      return Error(EndParen, "expected ')' in syncscope");
 
-    Scope = SynchronizationScope(ScopeU32);
+    Scope = SynchronizationScope(ScopeU8);
     return false;
   }
 

lib/AsmParser/LLParser.h

@@ -211,6 +211,11 @@ namespace llvm {
       return false;
     }
     bool ParseStringConstant(std::string &Result);
+    bool ParseUInt8(uint8_t &Val);
+    bool ParseUInt8(uint8_t &Val, LocTy &Loc) {
+      Loc = Lex.getLoc();
+      return ParseUInt8(Val);
+    }
     bool ParseUInt32(unsigned &Val);
     bool ParseUInt32(unsigned &Val, LocTy &Loc) {
       Loc = Lex.getLoc();

lib/AsmParser/LLToken.h

@@ -94,7 +94,7 @@ enum Kind {
   kw_acq_rel,
   kw_seq_cst,
   kw_singlethread,
-  kw_synchscope,
+  kw_syncscope,
 
   kw_nnan,
   kw_ninf,

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -970,12 +970,14 @@ static AtomicOrdering getDecodedOrdering(unsigned Val) {
 }
 
 static SynchronizationScope getDecodedSynchScope(unsigned Val) {
-  if (Val >= bitc::SYNCHSCOPE_FIRSTTARGETSPECIFIC)
+  if (Val >= bitc::SYNCHSCOPE_FIRSTTARGETSPECIFIC) {
+    assert(Val == uint8_t(Val) && "expected 8-bit integer (too large)");
     return SynchronizationScope(SynchronizationScopeFirstTargetSpecific +
-      (Val - bitc::SYNCHSCOPE_FIRSTTARGETSPECIFIC));
+        (Val - bitc::SYNCHSCOPE_FIRSTTARGETSPECIFIC));
+  }
 
   switch (Val) {
-  default: llvm_unreachable("Invalid synch scope");
+  default: llvm_unreachable("Invalid syncscope");
   case bitc::SYNCHSCOPE_SINGLETHREAD: return SingleThread;
   case bitc::SYNCHSCOPE_CROSSTHREAD: return CrossThread;
   }

lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -592,10 +592,10 @@ static unsigned getEncodedOrdering(AtomicOrdering Ordering) {
 static unsigned getEncodedSynchScope(SynchronizationScope SynchScope) {
   if (SynchScope >= SynchronizationScopeFirstTargetSpecific)
     return unsigned(bitc::SYNCHSCOPE_FIRSTTARGETSPECIFIC +
-      (SynchScope - SynchronizationScopeFirstTargetSpecific));
+        (SynchScope - SynchronizationScopeFirstTargetSpecific));
 
   switch (SynchScope) {
-  default: llvm_unreachable("Invalid synch scope");
+  default: llvm_unreachable("Invalid syncscope");
   case SingleThread: return bitc::SYNCHSCOPE_SINGLETHREAD;
   case CrossThread: return bitc::SYNCHSCOPE_CROSSTHREAD;
   }

lib/CodeGen/MachineInstr.cpp

@@ -542,13 +542,13 @@ MachineMemOperand::MachineMemOperand(MachinePointerInfo ptrinfo, Flags f,
                                      AtomicOrdering Ordering,
                                      AtomicOrdering FailureOrdering)
     : PtrInfo(ptrinfo), Size(s), FlagVals(f), BaseAlignLog2(Log2_32(a) + 1),
-      AAInfo(AAInfo), Ranges(Ranges), SynchScope(SynchScope),
-      Ordering(Ordering), FailureOrdering(FailureOrdering) {
+      AAInfo(AAInfo), Ranges(Ranges) {
   assert((PtrInfo.V.isNull() || PtrInfo.V.is<const PseudoSourceValue*>() ||
           isa<PointerType>(PtrInfo.V.get<const Value*>()->getType())) &&
          "invalid pointer value");
   assert(getBaseAlignment() == a && "Alignment is not a power of 2!");
   assert((isLoad() || isStore()) && "Not a load/store!");
+  InitAtomicInfo(SynchScope, Ordering, FailureOrdering);
 }
 
 /// Profile - Gather unique data for the object.