[PATCH] D99355: Implementation of intrinsic and SDNode definitions for VP load, store, gather, scatter.

[Hussain Kadhem via Phabricator via llvm-commits]

hussainjk added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SelectionDAGNodes.h:1367
     // with either an intrinsic or a target opcode.
-    return N->getOpcode() == ISD::LOAD                ||
-           N->getOpcode() == ISD::STORE               ||
-           N->getOpcode() == ISD::PREFETCH            ||
-           N->getOpcode() == ISD::ATOMIC_CMP_SWAP     ||
+    return N->getOpcode() == ISD::LOAD || N->getOpcode() == ISD::STORE ||
+           N->getOpcode() == ISD::PREFETCH ||
----------------
frasercrmck wrote:
> craig.topper wrote:
> > Can we override clang-format here? The previous code had a more consistent structure.
> Alternatively, I suppose, it could be a switch statement (aside from the last two)? Presumably that'd be optimized well enough by compilers?
It seems like this style is more idiomatic in the surrounding code, so leaving it as-is for now (with spacing fixed).



================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:402
 
+  /// \return the alignment of the pointer used by this load/store/gather or
+  /// scatter.
----------------
craig.topper wrote:
> It kind of feels like maybe there should be a VPMemIntrinsic and a VPStoreIntrinsic subclasses that contain these methods? Then you can use the more common isa, cast, dyn_cast and not have Optional return values.
There will be separate classes for the corresponding SDNodes at least, in the followup patch. A bunch of the code there also assumes you can generically call these methods, so I think it would be good to leave it like this for now and revisit it in the followup patch to minimize things breaking.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99355/new/

https://reviews.llvm.org/D99355