[PATCH] Masked Vector Load/Store Intrinsics

Elena Demikhovsky elena.demikhovsky at intel.com
Sat Nov 15 22:57:17 PST 2014


Thank you for reviewing. I fixed all comments / blank lines.

================
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
----------------
rob.khasanov wrote:
> 1) Comment doesn't correspond to class name (MaskedLoadSDNode vs. MaskedLoadStoreSDNode). 
> 2) Please remove one blank line before comment
done

================
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 {
+
----------------
rob.khasanov wrote:
> Correct ISD::MLOAD -> ISD::MSTORE in comment
done

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

================
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;
+}
+
+
----------------
rob.khasanov wrote:
> Will you change return values to PrevTTI->isLegalPredicatedLoad(...) in separate patch?
No. The default is always false.

================
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);
 
----------------
rob.khasanov wrote:
> I don't find definition of this function.
removed

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

================
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);
+
----------------
rob.khasanov wrote:
> Here should be: "store is independent of the other one"
fixed

http://reviews.llvm.org/D6191






More information about the llvm-commits mailing list