[PATCH] Masked Vector Load/Store Intrinsics

Demikhovsky, Elena elena.demikhovsky at intel.com
Tue Nov 18 05:27:53 PST 2014


Hi, please let me know whether I can commit the stuff.

Thank you.

-  Elena

-----Original Message-----
From: Elena Demikhovsky [mailto:elena.demikhovsky at intel.com] 
Sent: Sunday, November 16, 2014 08:57
To: Demikhovsky, Elena; anemet at apple.com; resistor at mac.com; hfinkel at anl.gov; rob.khasanov at gmail.com
Cc: peter_cooper at apple.com; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Masked Vector Load/Store Intrinsics

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


---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.




More information about the llvm-commits mailing list