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

[Craig Topper via Phabricator via llvm-commits]

craig.topper 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 ||
----------------
Can we override clang-format here? The previous code had a more consistent structure.


================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:402
 
+  /// \return the alignment of the pointer used by this load/store/gather or
+  /// scatter.
----------------
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.


================
Comment at: llvm/lib/IR/IntrinsicInst.cpp:299
+  Optional<int> PtrParamOpt = GetMemoryPointerParamPos(getIntrinsicID());
+  assert(PtrParamOpt.hasValue() && "no pointer argument!");
+  return this->getParamAlign(PtrParamOpt.getValue());
----------------
Doesn't the getValue() below already assert if there is no value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99355