[PATCH] D91441: [VP] Build VP SDNodes
Fraser Cormack via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 17 03:55:47 PST 2020
frasercrmck added inline comments.
================
Comment at: llvm/include/llvm/IR/VPIntrinsics.def:33
+// Register a new VP SDNode and begin its property scope.
+// When the SDNode scope is nested within a VP intrinsic scope, it is implicitly registered as the cannonical SDNode for this VP intrinsic.
+// There is one VP intrinsic that maps directly to one SDNode that goes by the
----------------
typo: `canonical`
================
Comment at: llvm/include/llvm/IR/VPIntrinsics.def:42
+// \p TDNAME The name of the TableGen definition of this SDNode.
+// name of the intrinsic and the TableGen def of the SDNode).
+// \p MASKPOS The mask operand position.
----------------
Missing `(` somewhere?
================
Comment at: llvm/include/llvm/IR/VPIntrinsics.def:44
+// \p MASKPOS The mask operand position.
+// \p EVLPOS The explicit vector length operand position.
+#ifndef BEGIN_REGISTER_VP_SDNODE
----------------
Mismatch between `EVLPOS` and `VLENPOS`
================
Comment at: llvm/include/llvm/IR/VPIntrinsics.def:72
+
+#define END_REGISTER_VP(INTRIN, SDID) \
+END_REGISTER_VP_INTRINSIC(INTRIN) \
----------------
I'm thinking of these as scopes as the documentation describes them, and it strikes me that the "INTRINSIC" and "SDNODE" scopes overlap here when using `BEGIN_REGISTER_VP` and `END_REGISTER_VP`:
```
BEGIN_REGISTER_VP_INTRINSIC
BEGIN_REGISTER_VP_SDNODE
END_REGISTER_VP_INTRINSIC
END_REGISTER_VP_SDNODE
```
Is there a reason for this, or should it be balanced?
```
BEGIN_REGISTER_VP_INTRINSIC
BEGIN_REGISTER_VP_SDNODE
END_REGISTER_VP_SDNODE
END_REGISTER_VP_INTRINSIC
```
================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:762
void visitConstrainedFPIntrinsic(const ConstrainedFPIntrinsic &FPI);
+ void visitVectorPredicationIntrinsic(const VPIntrinsic &FPI);
----------------
parameter named `VPIntrin` like in the function def?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91441/new/
https://reviews.llvm.org/D91441
More information about the llvm-commits
mailing list