[PATCH] D34629: Add isElementAtomic query method to MemInstrinsic class.

[Daniel Neilson via Phabricator via llvm-commits]

dneilson added a comment.

In https://reviews.llvm.org/D34629#797105, @efriedma wrote:

> Oh, I see...
>
> What's the advantage of "I->isElementAtomic()" over "isa<ElementUnorderedAtomicMemCpyInst>(I)"?
>
> Not sure it makes sense to make ElementUnorderedAtomicMemCpyInst inherit from MemCpyInst; it's likely to be confusing that a MemCpyInst might not really be a memcpy.


isa<> is possible, but then we'd probably have to have something like:

- MemIntrinsic
  - MemSetInst
  - MemTransferInst
    - MemCpyInst
    - MemMoveInst
  - ElementAtomicMemIntrinsic
    - ElementAtomicMemSetInst
    - ElementAtomicMemTransferInst
      - ElementAtomicMemCpyInst
      - ElementAtomicMemMoveInst

Would be more work updating passes, and will very likely result in a fair bit of code cloning. ex: When a pass's treatment of ElementAtomicMemTransferInst is identical to its treatment of MemTransferInst, and it's using a visitor to go through all instructions in a function/block/loop/etc. But, in exchange, we'd get a clear separation of concerns. Pros & cons to both options, I think.

Opinions?


https://reviews.llvm.org/D34629