[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 ||
----------------
hussainjk wrote:
> 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).
> 
Was this idiomatic comment referring to my comment or the switch suggestion?

If we don't do a switch I'd like to see it back the way it was originally so that each opcode is on one line. We can put clang-format off/on around it to suppress the clang format warning.


================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:409
+  Value *getMemoryPointerParam() const;
+  static Optional<int> getMemoryPointerParamPos(Intrinsic::ID);
+
----------------
This should return unsigned to match the recent change to GetVectorLengthParamPos and GetMaskParamPos.


================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:413
+  Value *getMemoryDataParam() const;
+  static Optional<int> getMemoryDataParamPos(Intrinsic::ID);
+
----------------
unsigned


================
Comment at: llvm/lib/IR/IntrinsicInst.cpp:298
+MaybeAlign VPIntrinsic::getPointerAlignment() const {
+  Optional<int> PtrParamOpt = getMemoryPointerParamPos(getIntrinsicID());
+  assert(PtrParamOpt.hasValue() && "no pointer argument!");
----------------
unsigned


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99355