[IR] Add support for memory attribute

[nikic]

Summary

This implements IR and bitcode support for the memory attribute, as proposed in https://discourse.llvm.org/t/rfc-unify-memory-effect-attributes/65579.

The new attribute is not used for anything yet (and as such, the old memory attributes are unaffected).

[nikic]

Print as memory(none) instead of memory() to make the meaning more obvious.

[nhaehnle]

Am I blind or does this not actually contain the bitcode parts yet? Generally looks fine to me at a glance, I expect you're going to rebase on top of the MemoryEffects rename.

[nikic]

Upload the correct patch... I accidentally replaced this with the next patch in the series.

[nikic]

In D135592#3861643, @nhaehnle wrote:

Am I blind or does this not actually contain the bitcode parts yet? Generally looks fine to me at a glance, I expect you're going to rebase on top of the MemoryEffects rename.

Ooops, I uploaded the wrong patch during the last update. Any yes, this will need to be adjusted for the MemoryEffects rename, and also for some of the LangRef changes (memory() should be rejected and the other location kind dropped).

[nikic]

Update according to LangRef changes, rebase over MemoryEffects rename.

[nikic]

I think this one is ready for review now.

[jdoerfert]

Looks good overall, the other uses confused me though.

llvm/lib/IR/Attributes.cpp

512

Here and in the examples, did we go back on the other front?

[nikic]

llvm/lib/IR/Attributes.cpp

512

other is not available in textual IR, but it is part of the in-memory representation -- we do need *some* way to internally represent everything that does not have a dedicated location kind.

[jdoerfert]

LG from my end. one nit and one suggestions. I doubt there is anything controversial so this should be OK to land.

llvm/lib/IR/Attributes.cpp

512

Fair, I got confused by the negative test case below.
That said, can we rename it? Default? Something other than other.
For one, "other" sounds odd, second, it's somewhat tainted now.
Anyway, that should not stop this patch, just a thought.

llvm/test/Assembler/memory-attribute-errors.ll

2

I didn't know this one, really cool!

llvm/test/Assembler/memory-attribute.ll

64

Add a test that reorders them too. I hope we emit a stable order.

llvm/include/llvm/AsmParser/LLParser.h

namespace llvm {
   class Value;
   class BasicBlock;
   class Instruction;
   class Constant;
   class GlobalValue;
   class Comdat;
   class MDString;
   class MDNode;
+  class MemoryEffects;
   struct SlotMapping;
 
   /// ValID - Represents a reference of a definition of some sort with no type.
   /// There are several cases where we have to parse the value but where the
   /// type can depend on later context.  This may either be a numeric reference
   /// or a symbolic (%var) reference.  This is just a discriminated union.
   struct ValID {
     enum {
private:
     void parseOptionalVisibility(unsigned &Res);
     void parseOptionalDLLStorageClass(unsigned &Res);
     bool parseOptionalCallingConv(unsigned &CC);
     bool parseOptionalAlignment(MaybeAlign &Alignment,
                                 bool AllowParens = false);
     bool parseOptionalDerefAttrBytes(lltok::Kind AttrKind, uint64_t &Bytes);
     bool parseOptionalUWTableKind(UWTableKind &Kind);
     bool parseAllocKind(AllocFnKind &Kind);
+    Optional<MemoryEffects> parseMemoryAttr();
     bool parseScopeAndOrdering(bool IsAtomic, SyncScope::ID &SSID,
                                AtomicOrdering &Ordering);
     bool parseScope(SyncScope::ID &SSID);
     bool parseOrdering(AtomicOrdering &Ordering);
     bool parseOptionalStackAlignment(unsigned &Alignment);
     bool parseOptionalCommaAlign(MaybeAlign &Alignment, bool &AteExtraComma);
     bool parseOptionalCommaAddrSpace(unsigned &AddrSpace, LocTy &Loc,
                                      bool &AteExtraComma);

llvm/include/llvm/AsmParser/LLToken.h

enum Kind {
   kw_attributes,
   kw_sync,
   kw_async,
 #define GET_ATTR_NAMES
 #define ATTRIBUTE_ENUM(ENUM_NAME, DISPLAY_NAME) \
   kw_##DISPLAY_NAME,
 #include "llvm/IR/Attributes.inc"
 
+  // Memory attribute:
+  kw_read,
+  kw_write,
+  kw_readwrite,
+  kw_argmem,
+  kw_inaccessiblemem,
+
   kw_type,
   kw_opaque,
 
   kw_comdat,
 
   // Comdat types
   kw_any,
   kw_exactmatch,

llvm/include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
   ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION = 78,
   ATTR_KIND_NO_SANITIZE_BOUNDS = 79,
   ATTR_KIND_ALLOC_ALIGN = 80,
   ATTR_KIND_ALLOCATED_POINTER = 81,
   ATTR_KIND_ALLOC_KIND = 82,
   ATTR_KIND_PRESPLIT_COROUTINE = 83,
   ATTR_KIND_FNRETTHUNK_EXTERN = 84,
   ATTR_KIND_SKIP_PROFILE = 85,
+  ATTR_KIND_MEMORY = 86,
 };
 
 enum ComdatSelectionKindCodes {
   COMDAT_SELECTION_KIND_ANY = 1,
   COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
   COMDAT_SELECTION_KIND_LARGEST = 3,
   COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
   COMDAT_SELECTION_KIND_SAME_SIZE = 5,

llvm/include/llvm/IR/Attributes.h

 class AttrBuilder;
 class AttributeMask;
 class AttributeImpl;
 class AttributeListImpl;
 class AttributeSetNode;
 class FoldingSetNodeID;
 class Function;
 class LLVMContext;
+class MemoryEffects;
 class Type;
 
 enum class AllocFnKind : uint64_t {
   Unknown = 0,
   Alloc = 1 << 0,         // Allocator function returns a new allocation
   Realloc = 1 << 1,       // Allocator function resizes the `allocptr` argument
   Free = 1 << 2,          // Allocator function frees the `allocptr` argument
   Uninitialized = 1 << 3, // Allocator function returns uninitialized memory
public:
   Optional<unsigned> getVScaleRangeMax() const;
 
   // Returns the unwind table kind.
   UWTableKind getUWTableKind() const;
 
   // Returns the allocator function kind.
   AllocFnKind getAllocKind() const;
 
+  /// Returns memory effects.
+  MemoryEffects getMemoryEffects() const;
+
   /// The Attribute is converted to a string of equivalent mnemonic. This
   /// is, presumably, for writing out the mnemonics for the assembly writer.
   std::string getAsString(bool InAttrGrp = false) const;
 
   /// Return true if this attribute belongs to the LLVMContext.
   bool hasParentContext(LLVMContext &C) const;
 
   /// Equality and non-equality operators.
public:
 
   /// This turns the unwind table kind into the form used internally in
   /// Attribute.
   AttrBuilder &addUWTableAttr(UWTableKind Kind);
 
   // This turns the allocator kind into the form used internally in Attribute.
   AttrBuilder &addAllocKindAttr(AllocFnKind Kind);
 
+  /// Add memory effect attribute.
+  AttrBuilder &addMemoryAttr(MemoryEffects ME);
+
   ArrayRef<Attribute> attrs() const { return Attrs; }
 
   bool operator==(const AttrBuilder &B) const;
   bool operator!=(const AttrBuilder &B) const { return !(*this == B); }
 };
 
 namespace AttributeFuncs {
 

llvm/include/llvm/IR/Attributes.td

 def InlineHint : EnumAttr<"inlinehint", [FnAttr]>;
 
 /// Force argument to be passed in register.
 def InReg : EnumAttr<"inreg", [ParamAttr, RetAttr]>;
 
 /// Build jump-instruction tables and replace refs.
 def JumpTable : EnumAttr<"jumptable", [FnAttr]>;
 
+/// Memory effects of the function.
+def Memory : IntAttr<"memory", [FnAttr]>;
+
 /// Function must be optimized for size first.
 def MinSize : EnumAttr<"minsize", [FnAttr]>;
 
 /// Naked function.
 def Naked : EnumAttr<"naked", [FnAttr]>;
 
 /// Nested function static chain.
 def Nest : EnumAttr<"nest", [ParamAttr]>;

llvm/include/llvm/IR/ModRef.h

private:
 
   static constexpr uint32_t BitsPerLoc = 2;
   static constexpr uint32_t LocMask = (1 << BitsPerLoc) - 1;
 
   static uint32_t getLocationPos(Location Loc) {
     return (uint32_t)Loc * BitsPerLoc;
   }
 
-  static auto locations() {
-    return enum_seq_inclusive(Location::ArgMem, Location::Other,
-                              force_iteration_on_noniterable_enum);
-  }
-
   MemoryEffects(uint32_t Data) : Data(Data) {}
 
   void setModRef(Location Loc, ModRefInfo MR) {
     Data &= ~(LocMask << getLocationPos(Loc));
     Data |= static_cast<uint32_t>(MR) << getLocationPos(Loc);
   }
 
   friend raw_ostream &operator<<(raw_ostream &OS, MemoryEffects RMRB);
 
 public:
+  /// Returns iterator over all supported location kinds.
+  static auto locations() {
+    return enum_seq_inclusive(Location::ArgMem, Location::Other,
+                              force_iteration_on_noniterable_enum);
+  }
+
   /// Create MemoryEffects that can access only the given location with the
   /// given ModRefInfo.
   MemoryEffects(Location Loc, ModRefInfo MR) { setModRef(Loc, MR); }
 
   /// Create MemoryEffects that can access any location with the given
   /// ModRefInfo.
   explicit MemoryEffects(ModRefInfo MR) {
     for (Location Loc : locations())
public:
   static MemoryEffects
   inaccessibleOrArgMemOnly(ModRefInfo MR = ModRefInfo::ModRef) {
     MemoryEffects FRMB = none();
     FRMB.setModRef(ArgMem, MR);
     FRMB.setModRef(InaccessibleMem, MR);
     return FRMB;
   }
 
+  /// Create MemoryEffects from an encoded integer value (used by memory
+  /// attribute).
+  static MemoryEffects createFromIntValue(uint32_t Data) {
+    return MemoryEffects(Data);
+  }
+
+  /// Convert MemoryEffects into an encoded integer value (used by memory
+  /// attribute).
+  uint32_t toIntValue() const {
+    return Data;
+  }
+
   /// Get ModRefInfo for the given Location.
   ModRefInfo getModRef(Location Loc) const {
     return ModRefInfo((Data >> getLocationPos(Loc)) & LocMask);
   }
 
   /// Get new MemoryEffects with modified ModRefInfo for Loc.
   MemoryEffects getWithModRef(Location Loc, ModRefInfo MR) const {
     MemoryEffects ME = *this;

llvm/lib/AsmParser/LLLexer.cpp

#define KEYWORD(STR)                                                           \
   KEYWORD(sync);
   KEYWORD(async);
 
 #define GET_ATTR_NAMES
 #define ATTRIBUTE_ENUM(ENUM_NAME, DISPLAY_NAME) \
   KEYWORD(DISPLAY_NAME);
 #include "llvm/IR/Attributes.inc"
 
+  KEYWORD(read);
+  KEYWORD(write);
+  KEYWORD(readwrite);
+  KEYWORD(argmem);
+  KEYWORD(inaccessiblemem);
+
   KEYWORD(type);
   KEYWORD(opaque);
 
   KEYWORD(comdat);
 
   // Comdat types
   KEYWORD(any);
   KEYWORD(exactmatch);

llvm/lib/AsmParser/LLParser.cpp

 //===-- LLParser.cpp - Parser Class ---------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the parser class for .ll files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/AsmParser/LLParser.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/AsmParser/LLToken.h"
 #include "llvm/AsmParser/SlotMapping.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Comdat.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #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"
+#include "llvm/IR/ModRef.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/ValueSymbolTable.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SaveAndRestore.h"
bool LLParser::parseEnumAttribute(Attribute::AttrKind Attr, AttrBuilder &B,
   }
   case Attribute::AllocKind: {
     AllocFnKind Kind = AllocFnKind::Unknown;
     if (parseAllocKind(Kind))
       return true;
     B.addAllocKindAttr(Kind);
     return false;
   }
+  case Attribute::Memory: {
+    Optional<MemoryEffects> ME = parseMemoryAttr();
+    if (!ME)
+      return true;
+    B.addMemoryAttr(*ME);
+    return false;
+  }
   default:
     B.addAttribute(Attr);
     Lex.Lex();
     return false;
   }
 }
 
 /// parseFnAttributeValuePairs
bool LLParser::parseAllocKind(AllocFnKind &Kind) {
   ParenLoc = Lex.getLoc();
   if (!EatIfPresent(lltok::rparen))
     return error(ParenLoc, "expected ')'");
   if (Kind == AllocFnKind::Unknown)
     return error(KindLoc, "expected allockind value");
   return false;
 }
 
+static Optional<MemoryEffects::Location> keywordToLoc(lltok::Kind Tok) {
+  switch (Tok) {
+  case lltok::kw_argmem:
+    return MemoryEffects::ArgMem;
+  case lltok::kw_inaccessiblemem:
+    return MemoryEffects::InaccessibleMem;
+  default:
+    return None;
+  }
+}
+
+static Optional<ModRefInfo> keywordToModRef(lltok::Kind Tok) {
+  switch (Tok) {
+  case lltok::kw_none:
+    return ModRefInfo::NoModRef;
+  case lltok::kw_read:
+    return ModRefInfo::Ref;
+  case lltok::kw_write:
+    return ModRefInfo::Mod;
+  case lltok::kw_readwrite:
+    return ModRefInfo::ModRef;
+  default:
+    return None;
+  }
+}
+
+Optional<MemoryEffects> LLParser::parseMemoryAttr() {
+  MemoryEffects ME = MemoryEffects::none();
+
+  // We use syntax like memory(argmem: read), so the colon should not be
+  // interpreted as a label terminator.
+  Lex.setIgnoreColonInIdentifiers(true);
+  auto _ = make_scope_exit([&] { Lex.setIgnoreColonInIdentifiers(false); });
+
+  Lex.Lex();
+  if (!EatIfPresent(lltok::lparen)) {
+    tokError("expected '('");
+    return None;
+  }
+
+  bool SeenLoc = false;
+  do {
+    Optional<MemoryEffects::Location> Loc = keywordToLoc(Lex.getKind());
+    if (Loc) {
+      Lex.Lex();
+      if (!EatIfPresent(lltok::colon)) {
+        tokError("expected ':' after location");
+        return None;
+      }
+    }
+
+    Optional<ModRefInfo> MR = keywordToModRef(Lex.getKind());
+    if (!MR) {
+      if (!Loc)
+        tokError("expected memory location (argmem, inaccessiblemem) "
+                 "or access kind (none, read, write, readwrite)");
+      else
+        tokError("expected access kind (none, read, write, readwrite)");
+      return None;
+    }
+
+    Lex.Lex();
+    if (Loc) {
+      SeenLoc = true;
+      ME = ME.getWithModRef(*Loc, *MR);
+    } else {
+      if (SeenLoc) {
+        tokError("default access kind must be specified first");
+        return None;
+      }
+      ME = MemoryEffects(*MR);
+    }
+
+    if (EatIfPresent(lltok::rparen))
+      return ME;
+  } while (EatIfPresent(lltok::comma));
+
+  tokError("unterminated memory attribute");
+  return None;
+}
+
 /// parseOptionalCommaAlign
 ///   ::=
 ///   ::= ',' align 4
 ///
 /// This returns with AteExtraComma set to true if it ate an excess comma at the
 /// end.
 bool LLParser::parseOptionalCommaAlign(MaybeAlign &Alignment,
                                        bool &AteExtraComma) {

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsAArch64.h"
 #include "llvm/IR/IntrinsicsARM.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Metadata.h"
+#include "llvm/IR/ModRef.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/Casting.h"
static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
   case bitc::ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY:
     return Attribute::InaccessibleMemOrArgMemOnly;
   case bitc::ATTR_KIND_INLINE_HINT:
     return Attribute::InlineHint;
   case bitc::ATTR_KIND_IN_REG:
     return Attribute::InReg;
   case bitc::ATTR_KIND_JUMP_TABLE:
     return Attribute::JumpTable;
+  case bitc::ATTR_KIND_MEMORY:
+    return Attribute::Memory;
   case bitc::ATTR_KIND_MIN_SIZE:
     return Attribute::MinSize;
   case bitc::ATTR_KIND_NAKED:
     return Attribute::Naked;
   case bitc::ATTR_KIND_NEST:
     return Attribute::Nest;
   case bitc::ATTR_KIND_NO_ALIAS:
     return Attribute::NoAlias;
case bitc::PARAMATTR_GRP_CODE_ENTRY: { // ENTRY: [grpid, idx, a0, a1, ...]
           else if (Kind == Attribute::AllocSize)
             B.addAllocSizeAttrFromRawRepr(Record[++i]);
           else if (Kind == Attribute::VScaleRange)
             B.addVScaleRangeAttrFromRawRepr(Record[++i]);
           else if (Kind == Attribute::UWTable)
             B.addUWTableAttr(UWTableKind(Record[++i]));
           else if (Kind == Attribute::AllocKind)
             B.addAllocKindAttr(static_cast<AllocFnKind>(Record[++i]));
+          else if (Kind == Attribute::Memory)
+            B.addMemoryAttr(MemoryEffects::createFromIntValue(Record[++i]));
         } else if (Record[i] == 3 || Record[i] == 4) { // String attribute
           bool HasValue = (Record[i++] == 4);
           SmallString<64> KindStr;
           SmallString<64> ValStr;
 
           while (Record[i] != 0 && i != e)
             KindStr += Record[i++];
           assert(Record[i] == 0 && "Kind string not null terminated");

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   case Attribute::JumpTable:
     return bitc::ATTR_KIND_JUMP_TABLE;
   case Attribute::MinSize:
     return bitc::ATTR_KIND_MIN_SIZE;
   case Attribute::AllocatedPointer:
     return bitc::ATTR_KIND_ALLOCATED_POINTER;
   case Attribute::AllocKind:
     return bitc::ATTR_KIND_ALLOC_KIND;
+  case Attribute::Memory:
+    return bitc::ATTR_KIND_MEMORY;
   case Attribute::Naked:
     return bitc::ATTR_KIND_NAKED;
   case Attribute::Nest:
     return bitc::ATTR_KIND_NEST;
   case Attribute::NoAlias:
     return bitc::ATTR_KIND_NO_ALIAS;
   case Attribute::NoBuiltin:
     return bitc::ATTR_KIND_NO_BUILTIN;

llvm/lib/IR/Attributes.cpp

 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/ModRef.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <cstdint>

 }
 
 AllocFnKind Attribute::getAllocKind() const {
   assert(hasAttribute(Attribute::AllocKind) &&
          "Trying to get allockind value from non-allockind attribute");
   return AllocFnKind(pImpl->getValueAsInt());
 }
 
+MemoryEffects Attribute::getMemoryEffects() const {
+  assert(hasAttribute(Attribute::Memory) &&
+         "Can only call getMemoryEffects() on memory attribute");
+  return MemoryEffects::createFromIntValue(pImpl->getValueAsInt());
+}
+
+static const char *getModRefStr(ModRefInfo MR) {
+  switch (MR) {
+  case ModRefInfo::NoModRef:
+    return "none";
+  case ModRefInfo::Ref:
+    return "read";
+  case ModRefInfo::Mod:
+    return "write";
+  case ModRefInfo::ModRef:
+    return "readwrite";
+  }
+  llvm_unreachable("Invalid ModRefInfo");
+}
+
 std::string Attribute::getAsString(bool InAttrGrp) const {
   if (!pImpl) return {};
 
   if (isEnumAttribute())
     return getNameFromAttrKind(getKindAsEnum()).str();
 
   if (isTypeAttribute()) {
     std::string Result = getNameFromAttrKind(getKindAsEnum()).str();
if ((Kind & AllocFnKind::Zeroed) != AllocFnKind::Unknown)
       parts.push_back("zeroed");
     if ((Kind & AllocFnKind::Aligned) != AllocFnKind::Unknown)
       parts.push_back("aligned");
     return ("allockind(\"" +
             Twine(llvm::join(parts.begin(), parts.end(), ",")) + "\")")
         .str();
   }
 
+  if (hasAttribute(Attribute::Memory)) {
+    std::string Result;
+    raw_string_ostream OS(Result);
+    bool First = true;
+    OS << "memory(";
+
+    MemoryEffects ME = getMemoryEffects();
+
+    // Print access kind for "other" as the default access kind. This way it
+    // will apply to any new location kinds that get split out of "other".
+    ModRefInfo OtherMR = ME.getModRef(MemoryEffects::Other);
+    if (OtherMR != ModRefInfo::NoModRef || ME.getModRef() == OtherMR) {
+      First = false;
+      OS << getModRefStr(OtherMR);
+    }
+
+    for (auto Loc : MemoryEffects::locations()) {
+      ModRefInfo MR = ME.getModRef(Loc);
+      if (MR == OtherMR)
+        continue;
+
+      if (!First)
+        OS << ", ";
+      First = false;
+
+      switch (Loc) {
+      case MemoryEffects::ArgMem:
+        OS << "argmem: ";
+        break;
+      case MemoryEffects::InaccessibleMem:
+        OS << "inaccessiblemem: ";
+        break;
+      case MemoryEffects::Other:
+        llvm_unreachable("This is represented as the default access kind");
+      }
+      OS << getModRefStr(MR);
+    }
+    OS << ")";
+    OS.flush();
+    return Result;
+  }
+
   // Convert target-dependent attributes to strings of the form:
   //
   //   "kind"
   //   "kind" = "value"
   //
   if (isStringAttribute()) {
     std::string Result;
     {

 }
 
 AttrBuilder &AttrBuilder::addUWTableAttr(UWTableKind Kind) {
   if (Kind == UWTableKind::None)
     return *this;
   return addRawIntAttr(Attribute::UWTable, uint64_t(Kind));
 }
 
+AttrBuilder &AttrBuilder::addMemoryAttr(MemoryEffects ME) {
+  return addRawIntAttr(Attribute::Memory, ME.toIntValue());
+}
+
 AttrBuilder &AttrBuilder::addAllocKindAttr(AllocFnKind Kind) {
   return addRawIntAttr(Attribute::AllocKind, static_cast<uint64_t>(Kind));
 }
 
 Type *AttrBuilder::getTypeAttr(Attribute::AttrKind Kind) const {
   assert(Attribute::isTypeAttrKind(Kind) && "Not a type attribute");
   Attribute A = getAttribute(Kind);
   return A.isValid() ? A.getValueAsType() : nullptr;

llvm/lib/Transforms/Utils/CodeExtractor.cpp

if (Attr.isStringAttribute()) {
       case Attribute::ReadOnly:
       case Attribute::ReturnsTwice:
       case Attribute::Speculatable:
       case Attribute::StackAlignment:
       case Attribute::WillReturn:
       case Attribute::WriteOnly:
       case Attribute::AllocKind:
       case Attribute::PresplitCoroutine:
+      case Attribute::Memory:
         continue;
       // Those attributes should be safe to propagate to the extracted function.
       case Attribute::AlwaysInline:
       case Attribute::Cold:
       case Attribute::DisableSanitizerInstrumentation:
       case Attribute::FnRetThunkExtern:
       case Attribute::Hot:
       case Attribute::NoRecurse:

llvm/test/Assembler/memory-attribute-errors.ll

+; RUN: split-file %s %t
+; RUN: not llvm-as < %t/missing-args.ll 2>&1 | FileCheck %s --check-prefix=MISSING-ARGS
+; RUN: not llvm-as < %t/empty.ll 2>&1 | FileCheck %s --check-prefix=EMPTY
+; RUN: not llvm-as < %t/unterminated.ll 2>&1 | FileCheck %s --check-prefix=UNTERMINATED
+; RUN: not llvm-as < %t/invalid-kind.ll 2>&1 | FileCheck %s --check-prefix=INVALID-KIND
+; RUN: not llvm-as < %t/other.ll 2>&1 | FileCheck %s --check-prefix=OTHER
+; RUN: not llvm-as < %t/missing-colon.ll 2>&1 | FileCheck %s --check-prefix=MISSING-COLON
+; RUN: not llvm-as < %t/invalid-access-kind.ll 2>&1 | FileCheck %s --check-prefix=INVALID-ACCESS-KIND
+; RUN: not llvm-as < %t/default-after-loc.ll 2>&1 | FileCheck %s --check-prefix=DEFAULT-AFTER-LOC
+
+;--- missing-args.ll
+; MISSING-ARGS: error: expected '('
+declare void @fn() memory
+;--- empty.ll
+; EMPTY: error: expected memory location (argmem, inaccessiblemem) or access kind (none, read, write, readwrite)
+declare void @fn() memory()
+;--- unterminated.ll
+; UNTERMINATED: error: unterminated memory attribute
+declare void @fn() memory(read
+;--- invalid-kind.ll
+; INVALID-KIND: error: expected memory location (argmem, inaccessiblemem) or access kind (none, read, write, readwrite)
+declare void @fn() memory(foo)
+;--- other.ll
+; OTHER: error: expected memory location (argmem, inaccessiblemem) or access kind (none, read, write, readwrite)
+declare void @fn() memory(other: read)
+;--- missing-colon.ll
+; MISSING-COLON: error: expected ':' after location
+declare void @fn() memory(argmem)
+;--- invalid-access-kind.ll
+; INVALID-ACCESS-KIND: error: expected access kind (none, read, write, readwrite)
+declare void @fn() memory(argmem: foo)
+;--- default-after-loc.ll
+; DEFAULT-AFTER-LOC: error: default access kind must be specified first
+declare void @fn() memory(argmem: read, write)

llvm/test/Assembler/memory-attribute.ll

+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
+; CHECK: Function Attrs: memory(none)
+; CHECK: @fn_readnone2()
+declare void @fn_readnone2() memory(none)
+
+; CHECK: Function Attrs: memory(read)
+; CHECK: @fn_readonly()
+declare void @fn_readonly() memory(read)
+
+; CHECK: Function Attrs: memory(write)
+; CHECK: @fn_writeonly()
+declare void @fn_writeonly() memory(write)
+
+; CHECK: Function Attrs: memory(readwrite)
+; CHECK: @fn_readwrite()
+declare void @fn_readwrite() memory(readwrite)
+
+; CHECK: Function Attrs: memory(argmem: read)
+; CHECK: @fn_argmem_read()
+declare void @fn_argmem_read() memory(argmem: read)
+
+; CHECK: Function Attrs: memory(argmem: write)
+; CHECK: @fn_argmem_write()
+declare void @fn_argmem_write() memory(argmem: write)
+
+; CHECK: Function Attrs: memory(argmem: readwrite)
+; CHECK: @fn_argmem_readwrite()
+declare void @fn_argmem_readwrite() memory(argmem: readwrite)
+
+; CHECK: Function Attrs: memory(inaccessiblemem: read)
+; CHECK: @fn_inaccessiblemem_read()
+declare void @fn_inaccessiblemem_read() memory(inaccessiblemem: read)
+
+; CHECK: Function Attrs: memory(inaccessiblemem: write)
+; CHECK: @fn_inaccessiblemem_write()
+declare void @fn_inaccessiblemem_write() memory(inaccessiblemem: write)
+
+; CHECK: Function Attrs: memory(inaccessiblemem: readwrite)
+; CHECK: @fn_inaccessiblemem_readwrite()
+declare void @fn_inaccessiblemem_readwrite() memory(inaccessiblemem: readwrite)
+
+; CHECK: Function Attrs: memory(read, argmem: readwrite)
+; CHECK: @fn_read_argmem_readwrite()
+declare void @fn_read_argmem_readwrite() memory(read, argmem: readwrite)
+
+; CHECK: Function Attrs: memory(read, argmem: write)
+; CHECK: @fn_read_argmem_write()
+declare void @fn_read_argmem_write() memory(read, argmem: write)
+
+; CHECK: Function Attrs: memory(read, argmem: none)
+; CHECK: @fn_read_argmem_none()
+declare void @fn_read_argmem_none() memory(read, argmem: none)
+
+; CHECK: Function Attrs: memory(argmem: read, inaccessiblemem: read)
+; CHECK: @fn_argmem_inaccessiblemem_read()
+declare void @fn_argmem_inaccessiblemem_read()
+    memory(argmem: read, inaccessiblemem: read)
+
+; CHECK: Function Attrs: memory(argmem: read, inaccessiblemem: write)
+; CHECK: @fn_argmem_read_inaccessiblemem_write()
+declare void @fn_argmem_read_inaccessiblemem_write()
+    memory(argmem: read, inaccessiblemem: write)
+
+; CHECK: Function Attrs: memory(argmem: read, inaccessiblemem: write)
+; CHECK: @fn_argmem_read_inaccessiblemem_write_reordered()
+declare void @fn_argmem_read_inaccessiblemem_write_reordered()
+    memory(inaccessiblemem: write, argmem: read)