Create instruction classes for identifying any atomicity of memory intrinsic. (NFC)

[dneilson]

Summary

For reference, see: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116589.html

This patch fleshes out the instruction class hierarchy with respect to atomic and
non-atomic memory intrinsics. With this change, the relevant part of the class
hierarchy becomes:

IntrinsicInst

-> MemIntrinsicBase (methods-only class)
  -> MemIntrinsic (non-atomic intrinsics)
    -> MemSetInst
    -> MemTransferInst
      -> MemCpyInst
      -> MemMoveInst
  -> AtomicMemIntrinsic (atomic intrinsics)
    -> AtomicMemSetInst
    -> AtomicMemTransferInst
      -> AtomicMemCpyInst
      -> AtomicMemMoveInst
  -> AnyMemIntrinsic (both atomicities)
    -> AnyMemSetInst
    -> AnyMemTransferInst
      -> AnyMemCpyInst
      -> AnyMemMoveInst

This involves some class renaming:

ElementUnorderedAtomicMemCpyInst -> AtomicMemCpyInst
ElementUnorderedAtomicMemMoveInst -> AtomicMemMoveInst
ElementUnorderedAtomicMemSetInst -> AtomicMemSetInst

A script for doing this renaming in downstream trees is included below.

An example of where the Any* classes should be used in LLVM is when reasoning
about the effects of an instruction (ex: aliasing).

---

Script for renaming AtomicMem* classes:
PREFIXES="[<,([:space:]]"
CLASSES="MemIntrinsic|MemTransferInst|MemSetInst|MemMoveInst|MemCpyInst"
SUFFIXES="[;)>,[:space:]]"

REGEX="(${PREFIXES})ElementUnorderedAtomic(${CLASSES})(${SUFFIXES})"
REGEX2="visitElementUnorderedAtomic(${CLASSES})"

FILES=$( grep -E "(${REGEX}|${REGEX2})" -r . | tr ':' ' ' | awk '{print $1}' | sort | uniq )

SED_SCRIPT="s~${REGEX}~\1Atomic\2\3~g"
SED_SCRIPT2="s~${REGEX2}~visitAtomic\1~g"

for f in $FILES; do

echo "Processing: $f"
sed  -i ".bak" -E "${SED_SCRIPT};${SED_SCRIPT2};${EA_SED_SCRIPT};${EA_SED_SCRIPT2}" $f

done

[dneilson]

Pulling this, for now. I'm going to make sure that the joint part of the hierarchy (i.e. the AnyMem* classes) will work before trying to land this part.

[hfinkel]

In D38419#884799, @dneilson wrote:

Pulling this, for now. I'm going to make sure that the joint part of the hierarchy (i.e. the AnyMem* classes) will work before trying to land this part.

FWIW, I don't really like the PainMem* names, although the AtomicMem* looks like an improvement. Can't you just leave the plain ones alone?

[dneilson]

In D38419#884865, @hfinkel wrote:

In D38419#884799, @dneilson wrote:

Pulling this, for now. I'm going to make sure that the joint part of the hierarchy (i.e. the AnyMem* classes) will work before trying to land this part.

FWIW, I don't really like the PainMem* names, although the AtomicMem* looks like an improvement. Can't you just leave the plain ones alone?

I have no particular objection to leaving the plain ones as is. It's what I suggested: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116716.html

A counter suggestion, that wasn't challenged, was made to change the plain ones as well: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116722.html

[hfinkel]

In D38419#884869, @dneilson wrote:

In D38419#884865, @hfinkel wrote:

In D38419#884799, @dneilson wrote:

Pulling this, for now. I'm going to make sure that the joint part of the hierarchy (i.e. the AnyMem* classes) will work before trying to land this part.

FWIW, I don't really like the PainMem* names, although the AtomicMem* looks like an improvement. Can't you just leave the plain ones alone?

I have no particular objection to leaving the plain ones as is. It's what I suggested: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116716.html

A counter suggestion, that wasn't challenged, was made to change the plain ones as well: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116722.html

Fair enough. I challenge :-) -- Sanjoy's comment about silent failures downstream is legitimate, but given how new the atomic versions of these are, I don't think that I'm concerned (and it will fail in the right way: by not recognizing the atomic forms, which is what any current code is probably intended to do (because it was written before the atomic forms existed)).

[dneilson]

In D38419#884894, @hfinkel wrote:

In D38419#884869, @dneilson wrote:

In D38419#884865, @hfinkel wrote:

In D38419#884799, @dneilson wrote:

Pulling this, for now. I'm going to make sure that the joint part of the hierarchy (i.e. the AnyMem* classes) will work before trying to land this part.

FWIW, I don't really like the PainMem* names, although the AtomicMem* looks like an improvement. Can't you just leave the plain ones alone?

I have no particular objection to leaving the plain ones as is. It's what I suggested: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116716.html

A counter suggestion, that wasn't challenged, was made to change the plain ones as well: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116722.html

Fair enough. I challenge :-) -- Sanjoy's comment about silent failures downstream is legitimate, but given how new the atomic versions of these are, I don't think that I'm concerned (and it will fail in the right way: by not recognizing the atomic forms, which is what any current code is probably intended to do (because it was written before the atomic forms existed)).

Playing a little devil's advocate... A benefit I can see, off the top of my head, in favour of changing to PlainMem* is cognitive. Having all of PlainMem* & AtomicMem* & AnyMem* makes the programmer have to think, briefly, about whether they're dealing with atomic or non-atomic variants. Leaving the non-atomic variants as is makes it easier to not even consider that the atomic variants exist.

[hfinkel]

In D38419#884898, @dneilson wrote:

In D38419#884894, @hfinkel wrote:

In D38419#884869, @dneilson wrote:

In D38419#884865, @hfinkel wrote:

In D38419#884799, @dneilson wrote:

Pulling this, for now. I'm going to make sure that the joint part of the hierarchy (i.e. the AnyMem* classes) will work before trying to land this part.

FWIW, I don't really like the PainMem* names, although the AtomicMem* looks like an improvement. Can't you just leave the plain ones alone?

I have no particular objection to leaving the plain ones as is. It's what I suggested: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116716.html

A counter suggestion, that wasn't challenged, was made to change the plain ones as well: http://lists.llvm.org/pipermail/llvm-dev/2017-August/116722.html

Fair enough. I challenge :-) -- Sanjoy's comment about silent failures downstream is legitimate, but given how new the atomic versions of these are, I don't think that I'm concerned (and it will fail in the right way: by not recognizing the atomic forms, which is what any current code is probably intended to do (because it was written before the atomic forms existed)).

Playing a little devil's advocate... A benefit I can see, off the top of my head, in favour of changing to PlainMem* is cognitive. Having all of PlainMem* & AtomicMem* & AnyMem* makes the programmer have to think, briefly, about whether they're dealing with atomic or non-atomic variants. Leaving the non-atomic variants as is makes it easier to not even consider that the atomic variants exist.

I know. Of course, trying to force people to think in this fashion doesn't always have the desired effect (doing the text substitution and moving on is more likely). It's also true that most people will never see these and we're not really giving people clear guidance on how to update their code. I think we can give relatively-clear guidance, something like, "If your code is looking at Mem* to reason about its effect on memory, then it can probably use AnyMem*, if you're transforming the Mem*, then you probably need to switch to using PlainMem* (or leave it alone, as the case may be). Frankly, I think we'll just need to be diligent in code review for a while regardless of what we do.

I'm also, in general, just not a fan of "Plain" as a prefix. Our memcpy intrinsics (and friends) are already extensions of the underlying C functions (we can specify alignments, volatileness, etc.). If we really want to rename in this way, I think that using NonAtomic as the prefix is better.

[dneilson]

Flush out the hierarchy, and do not rename the classes for the non-atomic memory intrinsics.

[dneilson]

ping - Could someone please take a look at this? Thank you.

[dneilson]

ping - Review still requested/needed. It may look daunting, but the change is pretty simple.

[dneilson]

• Rebasing.
• Removing change to AliasSetTracker; this makes this change purely NFC.

[sanjoy]

Hi Daniel,

I'll take a look at this today; but you might want to reply to the llvm-dev RFC thread with a link to this patch to get some more eyeballs on it.

[sanjoy]

Just to be clear, at the isa<>, dyn_cast<> level, this is the hierarchy you're shooting for right:

digraph G {
	AnyMemI -> MemI
	AnyMemI -> AtomicMemI
	AnyMemI -> AnyMemSetI
	AnyMemI -> AnyMemTransferI

	AnyMemSetI -> MemSetI
	AnyMemSetI -> AtomicMemSetI

	AnyMemTransferI -> AtomicMemTransferI
	AnyMemTransferI -> MemTransferI
	AnyMemTransferI -> AnyMemCpyI
	AnyMemTransferI -> AnyMemMoveI

	AtomicMemTransferI -> AtomicMemcpyI
	AtomicMemTransferI -> AtomicMemMoveI

	MemTransferI -> MemcpyI
	MemTransferI -> MemMoveI

	AnyMemCpyI -> MemcpyI
	AnyMemCpyI -> AtomicMemcpyI

	AnyMemMoveI -> MemMoveI
	AnyMemMoveI -> AtomicMemMoveI

	MemI -> MemTransferI
	MemI -> MemMoveI

	AtomicMemI -> AtomicMemTransferI
	AtomicMemI -> AtomicMemMoveI
}

include/llvm/IR/IntrinsicInst.h

218

This will let users up-cast AtomicMemInstrinsic and MemIntrinsic objects to MemIntrinsicBase objects -- is that okay? If it isn't, we can use CRTP to avoid giving AtomicMemInstrinsic and MemIntrinsic the same base class, as in:

template<typename Derived>
class MemIntrinsicBase : public IntrinsicInst { ... /* No real use of Derived anywhere */ };

class AtomicMemIntrinsic : public MemIntrinsicBase<AtomicMemIntrinsic> { ... };
class MemIntrinsic : public MemIntrinsicBase<MemIntrinsic> { ... };

// Now MemIntrinsic and AtomicMemIntrinsic do not have a common base class (at the C++ level).

237–238

Maybe explicitly note that this also strips addrspacecast?

319–320

The inline attribute is not necessary here I think -- member defined within the class are automatically inline.

401

Should we implement this for the atomic intrinsics as well?

520

Isn't the the common base for atomic *and non-atomic* mem(set|move|cpy)?

528

Did you consider not providing the isVolatile() accessor on AtomicMemIntrinsic? Design-wise I find that (not having the acccessor) slightly more preferable, but if that would require a lot of extra code then it isn't worth it.

627

These comments need to be updated.

[dneilson]

In D38419#907201, @sanjoy wrote:

Just to be clear, at the isa<>, dyn_cast<> level, this is the hierarchy you're shooting for right:

digraph G {
	AnyMemI -> MemI
	AnyMemI -> AtomicMemI
	AnyMemI -> AnyMemSetI
	AnyMemI -> AnyMemTransferI

	AnyMemSetI -> MemSetI
	AnyMemSetI -> AtomicMemSetI

	AnyMemTransferI -> AtomicMemTransferI
	AnyMemTransferI -> MemTransferI
	AnyMemTransferI -> AnyMemCpyI
	AnyMemTransferI -> AnyMemMoveI

	AtomicMemTransferI -> AtomicMemcpyI
	AtomicMemTransferI -> AtomicMemMoveI

	MemTransferI -> MemcpyI
	MemTransferI -> MemMoveI

	AnyMemCpyI -> MemcpyI
	AnyMemCpyI -> AtomicMemcpyI

	AnyMemMoveI -> MemMoveI
	AnyMemMoveI -> AtomicMemMoveI

	MemI -> MemTransferI
	MemI -> MemMoveI

	AtomicMemI -> AtomicMemTransferI
	AtomicMemI -> AtomicMemMoveI
}

The current isa<> hierarchy is this, minus the transitive edges:

digraph Existing {
  MemCpy -> MemTransfer
  MemMove -> MemTransfer
  MemTransfer -> MemI
  MemSet -> MemI
}

So, all of the following are true: isa<MemTransfer>(MemCpy), isa<MemTransfer>(MemMove), isa<MemI>(MemTransfer), and isa<MemI>(MemSet). Plus, the transitive relations: isa<MemI>(MemCpy), isa<MemI>(MemMove)

The new hierarchy is this exact same thing within each of of the atomicity-types (non-atomic, Atomic, and Any). In addition we have as true: isa<AnyX>(X) , isa<AnyX>(AtomicX), and all of the transitive edges like isa<AnyMemTransfer>(MemCpy) and isa<AnyMemI>(AtomicMemMove).

Stating the obvious... this is not a bidirectional relation. For example, it is not the case that isa<X>(AnyX) or isa<AtomicX>(AnyX).

include/llvm/IR/IntrinsicInst.h

218

I don't know that it actually matters whether they have a common C++ base class or not; I can't really see the base class ever being used outside of these class definitions.

CRTP seems to be more the norm in the LLVM codebase, so I could change this to use CRTP if only to be more consistent with other parts of LLVM.

I've no strong preference; I could go either way.

237–238

No harm in that.

401

The alignment stuff is trickier for the atomic memory intrinsics. Alignment is a property of the source & dest arguments, rather than an argument of its own. For the atomic intrinsics we'd need to implement getSourceAlignmentCst() and getDestAlignmentCst().

So, I was erring on the side of leaving it unimplemented for now and adding it in once we see whether it's useful and how it would be used.

Also, I'm hoping to get to revisiting the old patch that removes the alignment parameter from the non-atomic intrinsics and makes it an argument property. Once that's been done, then we could unify the alignment interfaces between atomic & non-atomic intrinsics.

528

Definitely considered it. I went for symmetry of implementation for no other reason than aesthetics.

Design-wise, there's definitely benefit to not having isVolatile() in the atomic hierarchy. Minimally, it's less confusing to implementers if it's not there -- since its always false for the atomic intrinsics.

If someone wants to add a volatile version of the atomic memory intrinsics down the line, for some reason, then they could always add isVolatile as part of that work.

[dneilson]

• Rebase.
• Fixing cut & paste errors in comments.
• Remove isVolatile() method from the atomic memory intrinsic hierarchy.
• Change MemInstrinsicBase to use CRTP.
• Remove inline keyword from classof() method declarations.

[sanjoy]

lgtm

I'd suggest waiting 1-2 days to give time for someone to chime in (after reading the llvm-dev ping you sent out) and then checking this in.

include/llvm/IR/IntrinsicInst.h

217

Please also add a comment on why we have CRTP here.

218

I don't know that it actually matters whether they have a common C++ base class or not; I can't really see the base class ever being used outside of these class definitions.

Another way of stating what I meant to say is that the fact that there is a MemIntrinsicBase class that is the base of all these classes is an implementation detail (I assumed this, maybe you have a different picture in mind); and for this reason we should help users not rely on this property by not actually providing a common base class.

401

SGTM

528

Minor nit: you could just use one dyn_cast here:

if (auto *MI = dyn_cast<MemIntrinsic>(...))
  return MI->isVolatile();

lib/IR/Verifier.cpp

4032

Mind removing this extra semi-colon as well?

[dneilson]

• Address minor nits.
• Clang-formatting change.

[dneilson]

• Missed a comment regarding why we use CRTP for the MemIntrinsicBase class.

include/llvm/IR/IntrinsicInst.h

static bool classof(const IntrinsicInst *I) {
       default: return false;
       }
     }
     static bool classof(const Value *V) {
       return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
     }
   };
 
-  /// This class represents atomic memcpy intrinsic
-  /// TODO: Integrate this class into MemIntrinsic hierarchy; for now this is
-  /// C&P of all methods from that hierarchy
-  class ElementUnorderedAtomicMemCpyInst : public IntrinsicInst {
+  /// Common base class for all memory intrinsics. Simply provides
+  /// common methods.
+  /// Written as CRTP to avoid a common base class amongst the
+  /// three atomicity hierarchies.
+  template <typename Derived> class MemIntrinsicBase : public IntrinsicInst {
   private:
-    enum { ARG_DEST = 0, ARG_SOURCE = 1, ARG_LENGTH = 2, ARG_ELEMENTSIZE = 3 };
+    enum { ARG_DEST = 0, ARG_LENGTH = 2 };
 
   public:
     Value *getRawDest() const {
       return const_cast<Value *>(getArgOperand(ARG_DEST));
     }
     const Use &getRawDestUse() const { return getArgOperandUse(ARG_DEST); }
     Use &getRawDestUse() { return getArgOperandUse(ARG_DEST); }
 
-    /// Return the arguments to the instruction.
-    Value *getRawSource() const {
-      return const_cast<Value *>(getArgOperand(ARG_SOURCE));
-    }
-    const Use &getRawSourceUse() const { return getArgOperandUse(ARG_SOURCE); }
-    Use &getRawSourceUse() { return getArgOperandUse(ARG_SOURCE); }
-
     Value *getLength() const {
       return const_cast<Value *>(getArgOperand(ARG_LENGTH));
     }
     const Use &getLengthUse() const { return getArgOperandUse(ARG_LENGTH); }
     Use &getLengthUse() { return getArgOperandUse(ARG_LENGTH); }
 
-    bool isVolatile() const { return false; }
-
-    Value *getRawElementSizeInBytes() const {
-      return const_cast<Value *>(getArgOperand(ARG_ELEMENTSIZE));
-    }
-
-    ConstantInt *getElementSizeInBytesCst() const {
-      return cast<ConstantInt>(getRawElementSizeInBytes());
-    }
-
-    uint32_t getElementSizeInBytes() const {
-      return getElementSizeInBytesCst()->getZExtValue();
-    }
-
     /// This is just like getRawDest, but it strips off any cast
-    /// instructions that feed it, giving the original input.  The returned
-    /// value is guaranteed to be a pointer.
+    /// instructions (including addrspacecast) that feed it, giving the
+    /// original input.  The returned value is guaranteed to be a pointer.
     Value *getDest() const { return getRawDest()->stripPointerCasts(); }
 
-    /// This is just like getRawSource, but it strips off any cast
-    /// instructions that feed it, giving the original input.  The returned
-    /// value is guaranteed to be a pointer.
-    Value *getSource() const { return getRawSource()->stripPointerCasts(); }
-
     unsigned getDestAddressSpace() const {
       return cast<PointerType>(getRawDest()->getType())->getAddressSpace();
     }
 
-    unsigned getSourceAddressSpace() const {
-      return cast<PointerType>(getRawSource()->getType())->getAddressSpace();
-    }
-
     /// Set the specified arguments of the instruction.
     void setDest(Value *Ptr) {
       assert(getRawDest()->getType() == Ptr->getType() &&
              "setDest called with pointer of wrong type!");
       setArgOperand(ARG_DEST, Ptr);
     }
 
-    void setSource(Value *Ptr) {
-      assert(getRawSource()->getType() == Ptr->getType() &&
-             "setSource called with pointer of wrong type!");
-      setArgOperand(ARG_SOURCE, Ptr);
-    }
-
     void setLength(Value *L) {
       assert(getLength()->getType() == L->getType() &&
              "setLength called with value of wrong type!");
       setArgOperand(ARG_LENGTH, L);
     }
-
-    void setElementSizeInBytes(Constant *V) {
-      assert(V->getType() == Type::getInt8Ty(getContext()) &&
-             "setElementSizeInBytes called with value of wrong type!");
-      setArgOperand(ARG_ELEMENTSIZE, V);
-    }
-
-    static bool classof(const IntrinsicInst *I) {
-      return I->getIntrinsicID() == Intrinsic::memcpy_element_unordered_atomic;
-    }
-    static bool classof(const Value *V) {
-      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
-    }
   };
 
-  class ElementUnorderedAtomicMemMoveInst : public IntrinsicInst {
+  // The common base class for the atomic memset/memmove/memcpy intrinsics
+  // i.e. llvm.element.unordered.atomic.memset/memcpy/memmove
+  class AtomicMemIntrinsic : public MemIntrinsicBase<AtomicMemIntrinsic> {
   private:
-    enum { ARG_DEST = 0, ARG_SOURCE = 1, ARG_LENGTH = 2, ARG_ELEMENTSIZE = 3 };
+    enum { ARG_ELEMENTSIZE = 3 };
 
   public:
-    Value *getRawDest() const {
-      return const_cast<Value *>(getArgOperand(ARG_DEST));
-    }
-    const Use &getRawDestUse() const { return getArgOperandUse(ARG_DEST); }
-    Use &getRawDestUse() { return getArgOperandUse(ARG_DEST); }
-
-    /// Return the arguments to the instruction.
-    Value *getRawSource() const {
-      return const_cast<Value *>(getArgOperand(ARG_SOURCE));
-    }
-    const Use &getRawSourceUse() const { return getArgOperandUse(ARG_SOURCE); }
-    Use &getRawSourceUse() { return getArgOperandUse(ARG_SOURCE); }
-
-    Value *getLength() const {
-      return const_cast<Value *>(getArgOperand(ARG_LENGTH));
-    }
-    const Use &getLengthUse() const { return getArgOperandUse(ARG_LENGTH); }
-    Use &getLengthUse() { return getArgOperandUse(ARG_LENGTH); }
-
-    bool isVolatile() const { return false; }
-
     Value *getRawElementSizeInBytes() const {
       return const_cast<Value *>(getArgOperand(ARG_ELEMENTSIZE));
     }
 
     ConstantInt *getElementSizeInBytesCst() const {
       return cast<ConstantInt>(getRawElementSizeInBytes());
     }
 
     uint32_t getElementSizeInBytes() const {
       return getElementSizeInBytesCst()->getZExtValue();
     }
 
-    /// This is just like getRawDest, but it strips off any cast
-    /// instructions that feed it, giving the original input.  The returned
-    /// value is guaranteed to be a pointer.
-    Value *getDest() const { return getRawDest()->stripPointerCasts(); }
-
-    /// This is just like getRawSource, but it strips off any cast
-    /// instructions that feed it, giving the original input.  The returned
-    /// value is guaranteed to be a pointer.
-    Value *getSource() const { return getRawSource()->stripPointerCasts(); }
-
-    unsigned getDestAddressSpace() const {
-      return cast<PointerType>(getRawDest()->getType())->getAddressSpace();
-    }
-
-    unsigned getSourceAddressSpace() const {
-      return cast<PointerType>(getRawSource()->getType())->getAddressSpace();
-    }
-
-    /// Set the specified arguments of the instruction.
-    void setDest(Value *Ptr) {
-      assert(getRawDest()->getType() == Ptr->getType() &&
-             "setDest called with pointer of wrong type!");
-      setArgOperand(ARG_DEST, Ptr);
-    }
-
-    void setSource(Value *Ptr) {
-      assert(getRawSource()->getType() == Ptr->getType() &&
-             "setSource called with pointer of wrong type!");
-      setArgOperand(ARG_SOURCE, Ptr);
-    }
-
-    void setLength(Value *L) {
-      assert(getLength()->getType() == L->getType() &&
-             "setLength called with value of wrong type!");
-      setArgOperand(ARG_LENGTH, L);
-    }
-
     void setElementSizeInBytes(Constant *V) {
       assert(V->getType() == Type::getInt8Ty(getContext()) &&
              "setElementSizeInBytes called with value of wrong type!");
       setArgOperand(ARG_ELEMENTSIZE, V);
     }
 
-    static inline bool classof(const IntrinsicInst *I) {
-      return I->getIntrinsicID() == Intrinsic::memmove_element_unordered_atomic;
+    static bool classof(const IntrinsicInst *I) {
+      switch (I->getIntrinsicID()) {
+      case Intrinsic::memcpy_element_unordered_atomic:
+      case Intrinsic::memmove_element_unordered_atomic:
+      case Intrinsic::memset_element_unordered_atomic:
+        return true;
+      default:
+        return false;
+      }
     }
-    static inline bool classof(const Value *V) {
+    static bool classof(const Value *V) {
       return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
     }
   };
 
   /// This class represents atomic memset intrinsic
-  /// TODO: Integrate this class into MemIntrinsic hierarchy; for now this is
-  /// C&P of all methods from that hierarchy
-  class ElementUnorderedAtomicMemSetInst : public IntrinsicInst {
+  // i.e. llvm.element.unordered.atomic.memset
+  class AtomicMemSetInst : public AtomicMemIntrinsic {
   private:
-    enum { ARG_DEST = 0, ARG_VALUE = 1, ARG_LENGTH = 2, ARG_ELEMENTSIZE = 3 };
+    enum { ARG_VALUE = 1 };
 
   public:
-    Value *getRawDest() const {
-      return const_cast<Value *>(getArgOperand(ARG_DEST));
+    Value *getValue() const {
+      return const_cast<Value *>(getArgOperand(ARG_VALUE));
     }
-    const Use &getRawDestUse() const { return getArgOperandUse(ARG_DEST); }
-    Use &getRawDestUse() { return getArgOperandUse(ARG_DEST); }
-
-    Value *getValue() const { return const_cast<Value*>(getArgOperand(ARG_VALUE)); }
     const Use &getValueUse() const { return getArgOperandUse(ARG_VALUE); }
     Use &getValueUse() { return getArgOperandUse(ARG_VALUE); }
 
-    Value *getLength() const {
-      return const_cast<Value *>(getArgOperand(ARG_LENGTH));
+    void setValue(Value *Val) {
+      assert(getValue()->getType() == Val->getType() &&
+             "setValue called with value of wrong type!");
+      setArgOperand(ARG_VALUE, Val);
     }
-    const Use &getLengthUse() const { return getArgOperandUse(ARG_LENGTH); }
-    Use &getLengthUse() { return getArgOperandUse(ARG_LENGTH); }
-
-    bool isVolatile() const { return false; }
 
-    Value *getRawElementSizeInBytes() const {
-      return const_cast<Value *>(getArgOperand(ARG_ELEMENTSIZE));
+    static bool classof(const IntrinsicInst *I) {
+      return I->getIntrinsicID() == Intrinsic::memset_element_unordered_atomic;
     }
-
-    ConstantInt *getElementSizeInBytesCst() const {
-      return cast<ConstantInt>(getRawElementSizeInBytes());
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
     }
+  };
 
-    uint32_t getElementSizeInBytes() const {
-      return getElementSizeInBytesCst()->getZExtValue();
+  // This class wraps the atomic memcpy/memmove intrinsics
+  // i.e. llvm.element.unordered.atomic.memcpy/memmove
+  class AtomicMemTransferInst : public AtomicMemIntrinsic {
+  private:
+    enum { ARG_SOURCE = 1 };
+
+  public:
+    /// Return the arguments to the instruction.
+    Value *getRawSource() const {
+      return const_cast<Value *>(getArgOperand(ARG_SOURCE));
     }
+    const Use &getRawSourceUse() const { return getArgOperandUse(ARG_SOURCE); }
+    Use &getRawSourceUse() { return getArgOperandUse(ARG_SOURCE); }
 
-    /// This is just like getRawDest, but it strips off any cast
+    /// This is just like getRawSource, but it strips off any cast
     /// instructions that feed it, giving the original input.  The returned
     /// value is guaranteed to be a pointer.
-    Value *getDest() const { return getRawDest()->stripPointerCasts(); }
+    Value *getSource() const { return getRawSource()->stripPointerCasts(); }
 
-    unsigned getDestAddressSpace() const {
-      return cast<PointerType>(getRawDest()->getType())->getAddressSpace();
+    unsigned getSourceAddressSpace() const {
+      return cast<PointerType>(getRawSource()->getType())->getAddressSpace();
     }
 
-    /// Set the specified arguments of the instruction.
-    void setDest(Value *Ptr) {
-      assert(getRawDest()->getType() == Ptr->getType() &&
-             "setDest called with pointer of wrong type!");
-      setArgOperand(ARG_DEST, Ptr);
+    void setSource(Value *Ptr) {
+      assert(getRawSource()->getType() == Ptr->getType() &&
+             "setSource called with pointer of wrong type!");
+      setArgOperand(ARG_SOURCE, Ptr);
     }
 
-    void setValue(Value *Val) {
-      assert(getValue()->getType() == Val->getType() &&
-             "setValue called with value of wrong type!");
-      setArgOperand(ARG_VALUE, Val);
+    static bool classof(const IntrinsicInst *I) {
+      switch (I->getIntrinsicID()) {
+      case Intrinsic::memcpy_element_unordered_atomic:
+      case Intrinsic::memmove_element_unordered_atomic:
+        return true;
+      default:
+        return false;
       }
-
-    void setLength(Value *L) {
-      assert(getLength()->getType() == L->getType() &&
-             "setLength called with value of wrong type!");
-      setArgOperand(ARG_LENGTH, L);
     }
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
 
-    void setElementSizeInBytes(Constant *V) {
-      assert(V->getType() == Type::getInt8Ty(getContext()) &&
-             "setElementSizeInBytes called with value of wrong type!");
-      setArgOperand(ARG_ELEMENTSIZE, V);
+  /// This class represents the atomic memcpy intrinsic
+  /// i.e. llvm.element.unordered.atomic.memcpy
+  class AtomicMemCpyInst : public AtomicMemTransferInst {
+  public:
+    static bool classof(const IntrinsicInst *I) {
+      return I->getIntrinsicID() == Intrinsic::memcpy_element_unordered_atomic;
+    }
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
     }
+  };
 
-    static inline bool classof(const IntrinsicInst *I) {
-      return I->getIntrinsicID() == Intrinsic::memset_element_unordered_atomic;
+  /// This class represents the atomic memmove intrinsic
+  /// i.e. llvm.element.unordered.atomic.memmove
+  class AtomicMemMoveInst : public AtomicMemTransferInst {
+  public:
+    static bool classof(const IntrinsicInst *I) {
+      return I->getIntrinsicID() == Intrinsic::memmove_element_unordered_atomic;
     }
-    static inline bool classof(const Value *V) {
+    static bool classof(const Value *V) {
       return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
     }
   };
 
   /// This is the common base class for memset/memcpy/memmove.
-  class MemIntrinsic : public IntrinsicInst {
-  public:
-    Value *getRawDest() const { return const_cast<Value*>(getArgOperand(0)); }
-    const Use &getRawDestUse() const { return getArgOperandUse(0); }
-    Use &getRawDestUse() { return getArgOperandUse(0); }
-
-    Value *getLength() const { return const_cast<Value*>(getArgOperand(2)); }
-    const Use &getLengthUse() const { return getArgOperandUse(2); }
-    Use &getLengthUse() { return getArgOperandUse(2); }
+  class MemIntrinsic : public MemIntrinsicBase<MemIntrinsic> {
+  private:
+    enum { ARG_ALIGN = 3, ARG_VOLATILE = 4 };
 
+  public:
     ConstantInt *getAlignmentCst() const {
-      return cast<ConstantInt>(const_cast<Value*>(getArgOperand(3)));
+      return cast<ConstantInt>(const_cast<Value *>(getArgOperand(ARG_ALIGN)));
     }
 
     unsigned getAlignment() const {
       return getAlignmentCst()->getZExtValue();
     }
 
     ConstantInt *getVolatileCst() const {
-      return cast<ConstantInt>(const_cast<Value*>(getArgOperand(4)));
+      return cast<ConstantInt>(
+          const_cast<Value *>(getArgOperand(ARG_VOLATILE)));
     }
 
     bool isVolatile() const {
       return !getVolatileCst()->isZero();
     }
 
-    unsigned getDestAddressSpace() const {
-      return cast<PointerType>(getRawDest()->getType())->getAddressSpace();
-    }
-
-    /// This is just like getRawDest, but it strips off any cast
-    /// instructions that feed it, giving the original input.  The returned
-    /// value is guaranteed to be a pointer.
-    Value *getDest() const { return getRawDest()->stripPointerCasts(); }
-
-    /// Set the specified arguments of the instruction.
-    void setDest(Value *Ptr) {
-      assert(getRawDest()->getType() == Ptr->getType() &&
-             "setDest called with pointer of wrong type!");
-      setArgOperand(0, Ptr);
-    }
-
-    void setLength(Value *L) {
-      assert(getLength()->getType() == L->getType() &&
-             "setLength called with value of wrong type!");
-      setArgOperand(2, L);
-    }
+    void setAlignment(Constant *A) { setArgOperand(ARG_ALIGN, A); }
 
-    void setAlignment(Constant* A) {
-      setArgOperand(3, A);
-    }
-
-    void setVolatile(Constant* V) {
-      setArgOperand(4, V);
-    }
+    void setVolatile(Constant *V) { setArgOperand(ARG_VOLATILE, V); }
 
     Type *getAlignmentType() const {
-      return getArgOperand(3)->getType();
+      return getArgOperand(ARG_ALIGN)->getType();
     }
 
     // Methods for support type inquiry through isa, cast, and dyn_cast:
     static bool classof(const IntrinsicInst *I) {
       switch (I->getIntrinsicID()) {
       case Intrinsic::memcpy:
       case Intrinsic::memmove:
       case Intrinsic::memset:
public:
     static bool classof(const IntrinsicInst *I) {
       return I->getIntrinsicID() == Intrinsic::memmove;
     }
     static bool classof(const Value *V) {
       return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
     }
   };
 
+  // The common base class for any memset/memmove/memcpy intrinsics;
+  // whether they be atomic or non-atomic.
+  // i.e. llvm.element.unordered.atomic.memset/memcpy/memmove
+  //  and llvm.memset/memcpy/memmove
+  class AnyMemIntrinsic : public MemIntrinsicBase<AnyMemIntrinsic> {
+  public:
+    bool isVolatile() const {
+      // Only the non-atomic intrinsics can be volatile
+      if (auto *MI = dyn_cast<MemIntrinsic>(this))
+        return MI->isVolatile();
+      return false;
+    }
+
+    static bool classof(const IntrinsicInst *I) {
+      switch (I->getIntrinsicID()) {
+      case Intrinsic::memcpy:
+      case Intrinsic::memmove:
+      case Intrinsic::memset:
+      case Intrinsic::memcpy_element_unordered_atomic:
+      case Intrinsic::memmove_element_unordered_atomic:
+      case Intrinsic::memset_element_unordered_atomic:
+        return true;
+      default:
+        return false;
+      }
+    }
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
+
+  /// This class represents any memset intrinsic
+  // i.e. llvm.element.unordered.atomic.memset
+  // and  llvm.memset
+  class AnyMemSetInst : public AnyMemIntrinsic {
+  private:
+    enum { ARG_VALUE = 1 };
+
+  public:
+    Value *getValue() const {
+      return const_cast<Value *>(getArgOperand(ARG_VALUE));
+    }
+    const Use &getValueUse() const { return getArgOperandUse(ARG_VALUE); }
+    Use &getValueUse() { return getArgOperandUse(ARG_VALUE); }
+
+    void setValue(Value *Val) {
+      assert(getValue()->getType() == Val->getType() &&
+             "setValue called with value of wrong type!");
+      setArgOperand(ARG_VALUE, Val);
+    }
+
+    static bool classof(const IntrinsicInst *I) {
+      switch (I->getIntrinsicID()) {
+      case Intrinsic::memset:
+      case Intrinsic::memset_element_unordered_atomic:
+        return true;
+      default:
+        return false;
+      }
+    }
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
+
+  // This class wraps any memcpy/memmove intrinsics
+  // i.e. llvm.element.unordered.atomic.memcpy/memmove
+  // and  llvm.memcpy/memmove
+  class AnyMemTransferInst : public AnyMemIntrinsic {
+  private:
+    enum { ARG_SOURCE = 1 };
+
+  public:
+    /// Return the arguments to the instruction.
+    Value *getRawSource() const {
+      return const_cast<Value *>(getArgOperand(ARG_SOURCE));
+    }
+    const Use &getRawSourceUse() const { return getArgOperandUse(ARG_SOURCE); }
+    Use &getRawSourceUse() { return getArgOperandUse(ARG_SOURCE); }
+
+    /// This is just like getRawSource, but it strips off any cast
+    /// instructions that feed it, giving the original input.  The returned
+    /// value is guaranteed to be a pointer.
+    Value *getSource() const { return getRawSource()->stripPointerCasts(); }
+
+    unsigned getSourceAddressSpace() const {
+      return cast<PointerType>(getRawSource()->getType())->getAddressSpace();
+    }
+
+    void setSource(Value *Ptr) {
+      assert(getRawSource()->getType() == Ptr->getType() &&
+             "setSource called with pointer of wrong type!");
+      setArgOperand(ARG_SOURCE, Ptr);
+    }
+
+    static bool classof(const IntrinsicInst *I) {
+      switch (I->getIntrinsicID()) {
+      case Intrinsic::memcpy:
+      case Intrinsic::memmove:
+      case Intrinsic::memcpy_element_unordered_atomic:
+      case Intrinsic::memmove_element_unordered_atomic:
+        return true;
+      default:
+        return false;
+      }
+    }
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
+
+  /// This class represents any memcpy intrinsic
+  /// i.e. llvm.element.unordered.atomic.memcpy
+  ///  and llvm.memcpy
+  class AnyMemCpyInst : public AnyMemTransferInst {
+  public:
+    static bool classof(const IntrinsicInst *I) {
+      switch (I->getIntrinsicID()) {
+      case Intrinsic::memcpy:
+      case Intrinsic::memcpy_element_unordered_atomic:
+        return true;
+      default:
+        return false;
+      }
+    }
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
+
+  /// This class represents any memmove intrinsic
+  /// i.e. llvm.element.unordered.atomic.memmove
+  ///  and llvm.memmove
+  class AnyMemMoveInst : public AnyMemTransferInst {
+  public:
+    static bool classof(const IntrinsicInst *I) {
+      switch (I->getIntrinsicID()) {
+      case Intrinsic::memmove:
+      case Intrinsic::memmove_element_unordered_atomic:
+        return true;
+      default:
+        return false;
+      }
+    }
+    static bool classof(const Value *V) {
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
+
   /// This represents the llvm.va_start intrinsic.
   class VAStartInst : public IntrinsicInst {
   public:
     static bool classof(const IntrinsicInst *I) {
       return I->getIntrinsicID() == Intrinsic::vastart;
     }
     static bool classof(const Value *V) {
       return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

case Intrinsic::memmove: {
     bool isTC = I.isTailCall() && isInTailCallPosition(&I, DAG.getTarget());
     SDValue MM = DAG.getMemmove(getRoot(), sdl, Op1, Op2, Op3, Align, isVol,
                                 isTC, MachinePointerInfo(I.getArgOperand(0)),
                                 MachinePointerInfo(I.getArgOperand(1)));
     updateDAGForMaybeTailCall(MM);
     return nullptr;
   }
   case Intrinsic::memcpy_element_unordered_atomic: {
-    const ElementUnorderedAtomicMemCpyInst &MI =
-        cast<ElementUnorderedAtomicMemCpyInst>(I);
+    const AtomicMemCpyInst &MI = cast<AtomicMemCpyInst>(I);
     SDValue Dst = getValue(MI.getRawDest());
     SDValue Src = getValue(MI.getRawSource());
     SDValue Length = getValue(MI.getLength());
 
     // Emit a library call.
     TargetLowering::ArgListTy Args;
     TargetLowering::ArgListEntry Entry;
     Entry.Ty = DAG.getDataLayout().getIntPtrType(*DAG.getContext());
CLI.setDebugLoc(sdl).setChain(getRoot()).setLibCallee(
                               TLI.getPointerTy(DAG.getDataLayout())),
         std::move(Args));
 
     std::pair<SDValue, SDValue> CallResult = TLI.LowerCallTo(CLI);
     DAG.setRoot(CallResult.second);
     return nullptr;
   }
   case Intrinsic::memmove_element_unordered_atomic: {
-    auto &MI = cast<ElementUnorderedAtomicMemMoveInst>(I);
+    auto &MI = cast<AtomicMemMoveInst>(I);
     SDValue Dst = getValue(MI.getRawDest());
     SDValue Src = getValue(MI.getRawSource());
     SDValue Length = getValue(MI.getLength());
 
     // Emit a library call.
     TargetLowering::ArgListTy Args;
     TargetLowering::ArgListEntry Entry;
     Entry.Ty = DAG.getDataLayout().getIntPtrType(*DAG.getContext());
CLI.setDebugLoc(sdl).setChain(getRoot()).setLibCallee(
                               TLI.getPointerTy(DAG.getDataLayout())),
         std::move(Args));
 
     std::pair<SDValue, SDValue> CallResult = TLI.LowerCallTo(CLI);
     DAG.setRoot(CallResult.second);
     return nullptr;
   }
   case Intrinsic::memset_element_unordered_atomic: {
-    auto &MI = cast<ElementUnorderedAtomicMemSetInst>(I);
+    auto &MI = cast<AtomicMemSetInst>(I);
     SDValue Dst = getValue(MI.getRawDest());
     SDValue Val = getValue(MI.getValue());
     SDValue Length = getValue(MI.getLength());
 
     // Emit a library call.
     TargetLowering::ArgListTy Args;
     TargetLowering::ArgListEntry Entry;
     Entry.Ty = DAG.getDataLayout().getIntPtrType(*DAG.getContext());

lib/IR/Verifier.cpp

case Intrinsic::memset: {
     Assert(AlignCI->isZero() || AlignVal.isPowerOf2(),
            "alignment argument of memory intrinsics must be a power of 2", CS);
     Assert(isa<ConstantInt>(CS.getArgOperand(4)),
            "isvolatile argument of memory intrinsics must be a constant int",
            CS);
     break;
   }
   case Intrinsic::memcpy_element_unordered_atomic: {
-    const ElementUnorderedAtomicMemCpyInst *MI =
-        cast<ElementUnorderedAtomicMemCpyInst>(CS.getInstruction());
-    ;
+    const AtomicMemCpyInst *MI = cast<AtomicMemCpyInst>(CS.getInstruction());
 
     ConstantInt *ElementSizeCI =
         dyn_cast<ConstantInt>(MI->getRawElementSizeInBytes());
     Assert(ElementSizeCI,
            "element size of the element-wise unordered atomic memory "
            "intrinsic must be a constant int",
            CS);
     const APInt &ElementSizeVal = ElementSizeCI->getValue();
uint64_t DstAlignment = CS.getParamAlignment(0),
              SrcAlignment = CS.getParamAlignment(1);
     Assert(IsValidAlignment(DstAlignment),
            "incorrect alignment of the destination argument", CS);
     Assert(IsValidAlignment(SrcAlignment),
            "incorrect alignment of the source argument", CS);
     break;
   }
   case Intrinsic::memmove_element_unordered_atomic: {
-    auto *MI = cast<ElementUnorderedAtomicMemMoveInst>(CS.getInstruction());
+    auto *MI = cast<AtomicMemMoveInst>(CS.getInstruction());
 
     ConstantInt *ElementSizeCI =
         dyn_cast<ConstantInt>(MI->getRawElementSizeInBytes());
     Assert(ElementSizeCI,
            "element size of the element-wise unordered atomic memory "
            "intrinsic must be a constant int",
            CS);
     const APInt &ElementSizeVal = ElementSizeCI->getValue();
uint64_t DstAlignment = CS.getParamAlignment(0),
              SrcAlignment = CS.getParamAlignment(1);
     Assert(IsValidAlignment(DstAlignment),
            "incorrect alignment of the destination argument", CS);
     Assert(IsValidAlignment(SrcAlignment),
            "incorrect alignment of the source argument", CS);
     break;
   }
   case Intrinsic::memset_element_unordered_atomic: {
-    auto *MI = cast<ElementUnorderedAtomicMemSetInst>(CS.getInstruction());
+    auto *MI = cast<AtomicMemSetInst>(CS.getInstruction());
 
     ConstantInt *ElementSizeCI =
         dyn_cast<ConstantInt>(MI->getRawElementSizeInBytes());
     Assert(ElementSizeCI,
            "element size of the element-wise unordered atomic memory "
            "intrinsic must be a constant int",
            CS);
     const APInt &ElementSizeVal = ElementSizeCI->getValue();

lib/Transforms/InstCombine/InstCombineCalls.cpp

for (unsigned I = 0, E = V->getNumElements(); I != E; ++I) {
     bool Sign = V->getElementType()->isIntegerTy()
                     ? cast<ConstantInt>(Elt)->isNegative()
                     : cast<ConstantFP>(Elt)->isNegative();
     BoolVec.push_back(ConstantInt::get(BoolTy, Sign));
   }
   return ConstantVector::get(BoolVec);
 }
 
-Instruction *InstCombiner::SimplifyElementUnorderedAtomicMemCpy(
-    ElementUnorderedAtomicMemCpyInst *AMI) {
+Instruction *
+InstCombiner::SimplifyElementUnorderedAtomicMemCpy(AtomicMemCpyInst *AMI) {
   // Try to unfold this intrinsic into sequence of explicit atomic loads and
   // stores.
   // First check that number of elements is compile time constant.
   auto *LengthCI = dyn_cast<ConstantInt>(AMI->getLength());
   if (!LengthCI)
     return nullptr;
 
   // Check that there are not too many elements.
if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(II)) {
     } else if (MemSetInst *MSI = dyn_cast<MemSetInst>(MI)) {
       if (Instruction *I = SimplifyMemSet(MSI))
         return I;
     }
 
     if (Changed) return II;
   }
 
-  if (auto *AMI = dyn_cast<ElementUnorderedAtomicMemCpyInst>(II)) {
+  if (auto *AMI = dyn_cast<AtomicMemCpyInst>(II)) {
     if (Constant *C = dyn_cast<Constant>(AMI->getLength()))
       if (C->isNullValue())
         return eraseInstFromFunction(*AMI);
 
     if (Instruction *I = SimplifyElementUnorderedAtomicMemCpy(AMI))
       return I;
   }
 

lib/Transforms/InstCombine/InstCombineInternal.h

Instruction *OptAndOp(BinaryOperator *Op, ConstantInt *OpRHS,
                         ConstantInt *AndRHS, BinaryOperator &TheAnd);
 
   Value *insertRangeTest(Value *V, const APInt &Lo, const APInt &Hi,
                          bool isSigned, bool Inside);
   Instruction *PromoteCastOfAllocation(BitCastInst &CI, AllocaInst &AI);
   Instruction *MatchBSwap(BinaryOperator &I);
   bool SimplifyStoreAtEndOfBlock(StoreInst &SI);
 
-  Instruction *
-  SimplifyElementUnorderedAtomicMemCpy(ElementUnorderedAtomicMemCpyInst *AMI);
+  Instruction *SimplifyElementUnorderedAtomicMemCpy(AtomicMemCpyInst *AMI);
   Instruction *SimplifyMemTransfer(MemIntrinsic *MI);
   Instruction *SimplifyMemSet(MemSetInst *MI);
 
   Value *EvaluateInDifferentType(Value *V, Type *Ty, bool isSigned);
 
   /// \brief Returns a value X such that Val = X * Scale, or null if none.
   ///
   /// If the multiplication is known not to overflow then NoSignedWrap is set.
   Value *Descale(Value *Val, APInt Scale, bool &NoSignedWrap);
 };
 
 } // end namespace llvm
 
 #undef DEBUG_TYPE
 
 #endif // LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEINTERNAL_H