[PATCH] Masked Vector Load/Store Intrinsics

Robert Khasanov rob.khasanov at gmail.com
Fri Nov 14 03:46:16 PST 2014


Hi Elena,

================
Comment at: include/llvm/CodeGen/SelectionDAGNodes.h:1929-1934
@@ -1926,3 +1928,8 @@
   }
 };
 
+
+/// MaskedLoadSDNode - This class is used to represent ISD::MLOAD nodes.
+///
+class MaskedLoadStoreSDNode : public MemSDNode {
+  // Operands
----------------
1) Comment doesn't correspond to class name (MaskedLoadSDNode vs. MaskedLoadStoreSDNode). 
2) Please remove one blank line before comment

================
Comment at: include/llvm/CodeGen/SelectionDAGNodes.h:1977-1979
@@ +1976,5 @@
+
+/// MaskedStoreSDNode - This class is used to represent ISD::MLOAD nodes.
+///
+class MaskedStoreSDNode : public MaskedLoadStoreSDNode {
+
----------------
Correct ISD::MLOAD -> ISD::MSTORE in comment

================
Comment at: include/llvm/CodeGen/SelectionDAGNodes.h:1995-1997
@@ -1929,1 +1994,5 @@
+  }
+};
+
+
 /// MachineSDNode - An SDNode that represents everything that will be needed
----------------
Remove one blank line

================
Comment at: include/llvm/Target/TargetSelectionDAG.td:191-198
@@ -190,2 +190,10 @@
 
+def SDTMaskedStore: SDTypeProfile<0, 3, [       // masked store
+  SDTCisPtrTy<0>, SDTCisVec<1>, SDTCisVec<2>
+]>;
+
+def SDTMaskedLoad: SDTypeProfile<1, 3, [       // masked load
+  SDTCisVec<0>, SDTCisPtrTy<1>, SDTCisVec<2>, SDTCisSameAs<0, 3>
+]>;
+
 def SDTVecShuffle : SDTypeProfile<1, 2, [
----------------
I suggest to introduce a new Selection DAG Type Constraint that checks both operands have the same number of elements in vector.

================
Comment at: lib/Analysis/TargetTransformInfo.cpp:104-113
@@ -103,1 +103,12 @@
 
+bool TargetTransformInfo::isLegalPredicatedLoad(Type *DataType,
+                                                int Consecutive) const {
+  return false;
+}
+
+bool TargetTransformInfo::isLegalPredicatedStore(Type *DataType,
+                                                 int Consecutive) const {
+  return false;
+}
+
+
----------------
Will you change return values to PrevTTI->isLegalPredicatedLoad(...) in separate patch?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:573
@@ -570,2 +572,3 @@
   void SplitVecRes_InregOp(SDNode *N, SDValue &Lo, SDValue &Hi);
+  void SplitVecRes_IntrTernaryOp(SDNode *N, SDValue &Lo, SDValue &Hi);
 
----------------
I don't find definition of this function.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:606
@@ -600,2 +605,3 @@
   SDValue SplitVecOp_FP_ROUND(SDNode *N);
+  SDValue SplitVecOp_IntrOp(SDNode *N);
 
----------------
Also, I don't find definition of this function.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1506-1508
@@ +1505,5 @@
+
+  // Build a factor node to remember that this load is independent of the
+  // other one.
+  return DAG.getNode(ISD::TokenFactor, DL, MVT::Other, Lo, Hi);
+
----------------
Here should be: "store is independent of the other one"

http://reviews.llvm.org/D6191






More information about the llvm-commits mailing list