[PATCH] D38419: Rename instruction classes for memory intrinsics. (NFC)

[Daniel Neilson via Phabricator via llvm-commits]

dneilson added a comment.

In https://reviews.llvm.org/D38419#884894, @hfinkel wrote:

> In https://reviews.llvm.org/D38419#884869, @dneilson wrote:
>
> > In https://reviews.llvm.org/D38419#884865, @hfinkel wrote:
> >
> > > In https://reviews.llvm.org/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.


https://reviews.llvm.org/D38419