[PATCH] Code Generator for Gather and Scatter Intrinsics.

Ahmed Bougacha ahmed.bougacha at gmail.com
Tue Apr 28 10:48:14 PDT 2015


Skimming through, the logic seems sound; a few nits (and a scary missing break).

Thanks!
-Ahmed


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/CodeGen/SelectionDAGNodes.h:1988
@@ -1985,1 +1987,3 @@
 
+/// This is a base class is used to represent
+/// MGATHER and MSCATTER nodes
----------------
One "is" too many.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1353
@@ -1301,2 +1352,3 @@
     case ISD::MSTORE:
       Res = SplitVecOp_MSTORE(cast<MaskedStoreSDNode>(N), OpNo);
+    case ISD::MSCATTER:
----------------
missing break?

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3724-3727
@@ +3723,6 @@
+  SDValue Store;
+  if (isStore) // Store form
+    Store = DAG.getMaskedStore(getRoot(), sdl, Src0, getValue(BasePtr), Mask,
+                               VT, MMO, false);
+  else { // Scatter
+    if (!SingleBase) {
----------------
{ } for the "if" as well, perhaps?

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3775
@@ -3691,2 +3774,3 @@
 
+  Value *MemOpBasePtr = (isLoad || SingleBase) ? BasePtr : NULL;
   MachineMemOperand *MMO =
----------------
nullptr

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3783-3786
@@ +3782,6 @@
+  SDValue Load;
+  if (isLoad) // Load form
+    Load = DAG.getMaskedLoad(VT, sdl, Root, getValue(Ptr), Mask, Src0, VT, MMO,
+	                         ISD::NON_EXTLOAD);
+  else {
+    if (!SingleBase) {
----------------
Same, { } ?

================
Comment at: test/CodeGen/X86/masked_gather_scatter.ll:107-143
@@ +106,39 @@
+
+define <8 x i32> @test7(i32* %base, <8 x i32> %ind, i8 %mask) {
+
+  %broadcast.splatinsert = insertelement <8 x i32*> undef, i32* %base, i32 0
+  %broadcast.splat = shufflevector <8 x i32*> %broadcast.splatinsert, <8 x i32*> undef, <8 x i32> zeroinitializer
+
+  %gep.random = getelementptr i32, <8 x i32*> %broadcast.splat, <8 x i32> %ind
+  %imask = bitcast i8 %mask to <8 x i1>
+  %gt1 = call <8 x i32> @llvm.masked.gather.v8i32(<8 x i32*> %gep.random, i32 4, <8 x i1> %imask, <8 x i32>undef)
+  %gt2 = call <8 x i32> @llvm.masked.gather.v8i32(<8 x i32*> %gep.random, i32 4, <8 x i1> %imask, <8 x i32>%gt1)
+  %res = add <8 x i32> %gt1, %gt2
+  ret <8 x i32> %res
+}
+
+define <8 x i32> @test8(i8* %b, <8 x i32> %ind, i8 %mask) {
+  %base = bitcast i8* %b to i32*
+  br label %vector.body
+
+vector.body:                                      
+
+  %broadcast.splatinsert = insertelement <8 x i32*> undef, i32* %base, i32 0
+  %broadcast.splat = shufflevector <8 x i32*> %broadcast.splatinsert, <8 x i32*> undef, <8 x i32> zeroinitializer
+
+  %gep.random = getelementptr i32, <8 x i32*> %broadcast.splat, <8 x i32> %ind
+  %imask = bitcast i8 %mask to <8 x i1>
+  %gt1 = call <8 x i32> @llvm.masked.gather.v8i32(<8 x i32*> %gep.random, i32 4, <8 x i1> %imask, <8 x i32>undef)
+  %gt2 = call <8 x i32> @llvm.masked.gather.v8i32(<8 x i32*> %gep.random, i32 4, <8 x i1> %imask, <8 x i32>%gt1)
+  %res = add <8 x i32> %gt1, %gt2
+  ret <8 x i32> %res
+}
+
+define <16 x i32> @test9(<16 x i32*> %ptr.random, <16 x i32> %ind, i16 %mask) {
+  %imask = bitcast i16 %mask to <16 x i1>
+  %gt1 = call <16 x i32> @llvm.masked.gather.v16i32(<16 x i32*> %ptr.random, i32 4, <16 x i1> %imask, <16 x i32>undef)
+  %gt2 = call <16 x i32> @llvm.masked.gather.v16i32(<16 x i32*> %ptr.random, i32 4, <16 x i1> %imask, <16 x i32>%gt1)
+  %res = add <16 x i32> %gt1, %gt2
+  ret <16 x i32> %res
+}
+
----------------
Missing CHECKs ?

================
Comment at: test/CodeGen/X86/masked_gather_scatter.ll:144-147
@@ +143,5 @@
+}
+
+
+
+
----------------
Needless space?

================
Comment at: test/CodeGen/X86/masked_memop.ll:10-11
@@ -9,4 +9,4 @@
 ; AVX2-LABEL: test1
-; AVX2: vpmaskmovd      32(%rdi)
-; AVX2: vpmaskmovd      (%rdi)
+; AVX2: vpmaskmovd      {{.*}}(%rdi)
+; AVX2: vpmaskmovd      {{.*}}(%rdi)
 ; AVX2-NOT: blend
----------------
This is surprising, do we know why this happens?

http://reviews.llvm.org/D7665

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list