[ppc] Legalize the load of MVT::v4i8 into VSX register

[Carrot]

Summary

VSX has an instruction lxsiwax that can load 32bit value into VSX register. That patch makes it known to memory cost model, so the vectorization of the test case in pr30990 is beneficial.

[nemanjai]

lib/Target/PowerPC/PPCISelLowering.cpp

680 ↗

(On Diff #78092)

Is there more to this patch? Taking a quick look in PPCISelLowering.cpp, I don't see which custom lowering function will be invoked that will actually handle this node. Of course, it's entirely possible that I just missed it.

[Carrot]

lib/Target/PowerPC/PPCISelLowering.cpp

680 ↗

(On Diff #78092)

You didn't miss anything.
After vectorization following load instruction is generated
%wide.load = load <4 x i8>, <4 x i8>* %lsr.iv16, align 1, !tbaa !1

In lowering phase, it is translated to following SDNode
t5: v4i8,ch = load<LD4[%lsr.iv16](align=1)(tbaa=<0xa868c68>)> t0, t2, undef:i64

And after LegalizeTypes(),

    t48: i32,ch = load<LD4[%lsr.iv16](align=1)(tbaa=<0xa868c68>)> t0, t2, undef:i64
  t49: v4i32 = scalar_to_vector t48
t50: v16i8 = bitcast t49

And then it is directly translated to LXSIWAX.
So it is already correctly handled.

[nemanjai]

lib/Target/PowerPC/PPCISelLowering.cpp

680 ↗

(On Diff #78092)

I see, legalization eliminates this illegal type and this patch just serves to provide a better answer when TTI checks if this is "LegalOrCustom" so that the vectorizer does not consider this too expensive to be profitable.
If that understanding is correct, could you please add a comment outlining this above this line so that it is clear why this is a Custom operation?

[Carrot]

ping

[amehsan]

lib/Target/PowerPC/PPCISelLowering.cpp

681–683 ↗

(On Diff #78424)

Now that we are fixing this for v4i8, is it possible to fix this for some other vector types if a similar fix makes sense for them? For example does it make sense to make a similar change for v4i16 or v2i16 as well?

[hfinkel]

This is certainly a novel use of setLoadExtAction for an illegal type ;) -- I worry that this is a bit hacky and not a technique we can use universally regardless. My inclination is that putting logic into PPCTargetTransformInfo would be better. X86TargetTransformInfo certainly has its fair share of cost tables, etc. to handle various situations the generic cost logic doesn't get quite right. I'm certainly open to hearing other opinions.

[Carrot]

Add the memory cost computation to PPCTTIImpl::getMemoryOpCost.

[hfinkel]

Please also directly add a cost-model test (e.g. to test/Analysis/CostModel/PowerPC/load_store.ll). With that, this LGTM.

llvm/trunk/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

int PPCTTIImpl::getMemoryOpCost(unsigned Opcode, Type *Src, unsigned Alignment,
                                 unsigned AddressSpace) {
   // Legalize the type.
   std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, Src);
   assert((Opcode == Instruction::Load || Opcode == Instruction::Store) &&
          "Invalid Opcode");
 
   int Cost = BaseT::getMemoryOpCost(Opcode, Src, Alignment, AddressSpace);
 
-  // Aligned loads and stores are easy.
-  unsigned SrcBytes = LT.second.getStoreSize();
-  if (!SrcBytes || !Alignment || Alignment >= SrcBytes)
-    return Cost;
-
   bool IsAltivecType = ST->hasAltivec() &&
                        (LT.second == MVT::v16i8 || LT.second == MVT::v8i16 ||
                         LT.second == MVT::v4i32 || LT.second == MVT::v4f32);
   bool IsVSXType = ST->hasVSX() &&
                    (LT.second == MVT::v2f64 || LT.second == MVT::v2i64);
   bool IsQPXType = ST->hasQPX() &&
                    (LT.second == MVT::v4f64 || LT.second == MVT::v4f32);
 
+  // VSX has 32b/64b load instructions. Legalization can handle loading of
+  // 32b/64b to VSR correctly and cheaply. But BaseT::getMemoryOpCost and
+  // PPCTargetLowering can't compute the cost appropriately. So here we
+  // explicitly check this case.
+  unsigned MemBytes = Src->getPrimitiveSizeInBits();
+  if (Opcode == Instruction::Load && ST->hasVSX() && IsAltivecType &&
+      (MemBytes == 64 || (ST->hasP8Vector() && MemBytes == 32)))
+    return 1;
+
+  // Aligned loads and stores are easy.
+  unsigned SrcBytes = LT.second.getStoreSize();
+  if (!SrcBytes || !Alignment || Alignment >= SrcBytes)
+    return Cost;
+
   // If we can use the permutation-based load sequence, then this is also
   // relatively cheap (not counting loop-invariant instructions): one load plus
   // one permute (the last load in a series has extra cost, but we're
   // neglecting that here). Note that on the P7, we could do unaligned loads
   // for Altivec types using the VSX instructions, but that's more expensive
   // than using the permutation-based load sequence. On the P8, that's no
   // longer true.
   if (Opcode == Instruction::Load &&

llvm/trunk/test/Analysis/CostModel/PowerPC/vsr_load_32_64.ll

+; RUN: opt < %s -cost-model -analyze -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr8 -mattr=+vsx | FileCheck %s
+target datalayout = "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-f128:128:128-v128:128:128-n32:64"
+target triple = "powerpc64-unknown-linux-gnu"
+
+define i32 @loads(i32 %arg) {
+  ; CHECK: cost of 1 {{.*}} load
+  load <4 x i8>, <4 x i8>* undef, align 1
+
+  ; CHECK: cost of 1 {{.*}} load
+  load <8 x i8>, <8 x i8>* undef, align 1
+
+  ; CHECK: cost of 1 {{.*}} load
+  load <2 x i16>, <2 x i16>* undef, align 2
+
+  ; CHECK: cost of 1 {{.*}} load
+  load <4 x i16>, <4 x i16>* undef, align 2
+
+  ret i32 undef
+}

llvm/trunk/test/Transforms/LoopVectorize/PowerPC/pr30990.ll

+; RUN: opt < %s -loop-vectorize -mcpu=pwr8 -mattr=+vsx -force-vector-interleave=1 -vectorizer-maximize-bandwidth=0 -S | FileCheck %s
+
+target triple = "powerpc64-unknown-linux-gnu"
+
+define signext i32 @foo(i8* readonly %ptr, i32 signext %l) {
+entry:
+  %idx.ext = sext i32 %l to i64
+  %add.ptr = getelementptr inbounds i8, i8* %ptr, i64 %idx.ext
+  %cmp7 = icmp sgt i32 %l, 0
+  br i1 %cmp7, label %while.body.preheader, label %while.end
+
+while.body.preheader:                             ; preds = %entry
+  br label %while.body
+
+while.body:                                       ; preds = %while.body.preheader, %while.body
+  %count.09 = phi i32 [ %add, %while.body ], [ 0, %while.body.preheader ]
+  %ptr.addr.08 = phi i8* [ %incdec.ptr, %while.body ], [ %ptr, %while.body.preheader ]
+  %0 = load i8, i8* %ptr.addr.08, align 1
+  %cmp1 = icmp slt i8 %0, -64
+  %cond = zext i1 %cmp1 to i32
+  %add = add nsw i32 %cond, %count.09
+  %incdec.ptr = getelementptr inbounds i8, i8* %ptr.addr.08, i64 1
+  %cmp = icmp ult i8* %incdec.ptr, %add.ptr
+  br i1 %cmp, label %while.body, label %while.end.loopexit
+
+while.end.loopexit:                               ; preds = %while.body
+  %add.lcssa = phi i32 [ %add, %while.body ]
+  br label %while.end
+
+while.end:                                        ; preds = %while.end.loopexit, %entry
+  %count.0.lcssa = phi i32 [ 0, %entry ], [ %add.lcssa, %while.end.loopexit ]
+  ret i32 %count.0.lcssa
+
+; CHECK: load <4 x i8>
+; CHECK: icmp slt <4 x i8>
+}
+
+
+define signext i16 @foo2(i8* readonly %ptr, i32 signext %l) {
+entry:
+  %idx.ext = sext i32 %l to i64 
+  %add.ptr = getelementptr inbounds i8, i8* %ptr, i64 %idx.ext
+  %cmp7 = icmp sgt i32 %l, 0
+  br i1 %cmp7, label %while.body.preheader, label %while.end
+
+while.body.preheader:                             ; preds = %entry
+  br label %while.body
+
+while.body:                                       ; preds = %while.body.preheader, %while.body
+  %count.09 = phi i16 [ %add, %while.body ], [ 0, %while.body.preheader ]
+  %ptr.addr.08 = phi i8* [ %incdec.ptr, %while.body ], [ %ptr, %while.body.preheader ]
+  %0 = load i8, i8* %ptr.addr.08, align 1
+  %cmp1 = icmp slt i8 %0, -64 
+  %cond = zext i1 %cmp1 to i16 
+  %add = add nsw i16 %cond, %count.09
+  %incdec.ptr = getelementptr inbounds i8, i8* %ptr.addr.08, i64 1
+  %cmp = icmp ult i8* %incdec.ptr, %add.ptr
+  br i1 %cmp, label %while.body, label %while.end.loopexit
+
+while.end.loopexit:                               ; preds = %while.body
+  %add.lcssa = phi i16 [ %add, %while.body ]
+  br label %while.end
+
+while.end:                                        ; preds = %while.end.loopexit, %entry
+  %count.0.lcssa = phi i16 [ 0, %entry ], [ %add.lcssa, %while.end.loopexit ]
+  ret i16 %count.0.lcssa
+
+; CHECK-LABEL: foo2
+; CHECK: load <8 x i8>
+; CHECK: icmp slt <8 x i8>
+}
+
+define signext i32 @foo3(i16* readonly %ptr, i32 signext %l) {
+entry:
+  %idx.ext = sext i32 %l to i64 
+  %add.ptr = getelementptr inbounds i16, i16* %ptr, i64 %idx.ext
+  %cmp7 = icmp sgt i32 %l, 0
+  br i1 %cmp7, label %while.body.preheader, label %while.end
+
+while.body.preheader:                             ; preds = %entry
+  br label %while.body
+
+while.body:                                       ; preds = %while.body.preheader, %while.body
+  %count.09 = phi i32 [ %add, %while.body ], [ 0, %while.body.preheader ]
+  %ptr.addr.16 = phi i16* [ %incdec.ptr, %while.body ], [ %ptr, %while.body.preheader ]
+  %0 = load i16, i16* %ptr.addr.16, align 1
+  %cmp1 = icmp slt i16 %0, -64 
+  %cond = zext i1 %cmp1 to i32 
+  %add = add nsw i32 %cond, %count.09
+  %incdec.ptr = getelementptr inbounds i16, i16* %ptr.addr.16, i64 1
+  %cmp = icmp ult i16* %incdec.ptr, %add.ptr
+  br i1 %cmp, label %while.body, label %while.end.loopexit
+
+while.end.loopexit:                               ; preds = %while.body
+  %add.lcssa = phi i32 [ %add, %while.body ]
+  br label %while.end
+
+while.end:                                        ; preds = %while.end.loopexit, %entry
+  %count.0.lcssa = phi i32 [ 0, %entry ], [ %add.lcssa, %while.end.loopexit ]
+  ret i32 %count.0.lcssa
+
+; CHECK-LABEL: foo3
+; CHECK: load <4 x i16>
+; CHECK: icmp slt <4 x i16>
+}
+
+define i64 @foo4(i16* readonly %ptr, i32 signext %l) {
+entry:
+  %idx.ext = sext i32 %l to i64 
+  %add.ptr = getelementptr inbounds i16, i16* %ptr, i64 %idx.ext
+  %cmp7 = icmp sgt i32 %l, 0
+  br i1 %cmp7, label %while.body.preheader, label %while.end
+
+while.body.preheader:                             ; preds = %entry
+  br label %while.body
+
+while.body:                                       ; preds = %while.body.preheader, %while.body
+  %count.09 = phi i64 [ %add, %while.body ], [ 0, %while.body.preheader ]
+  %ptr.addr.16 = phi i16* [ %incdec.ptr, %while.body ], [ %ptr, %while.body.preheader ]
+  %0 = load i16, i16* %ptr.addr.16, align 1
+  %cmp1 = icmp slt i16 %0, -64 
+  %cond = zext i1 %cmp1 to i64 
+  %add = add nsw i64 %cond, %count.09
+  %incdec.ptr = getelementptr inbounds i16, i16* %ptr.addr.16, i64 1
+  %cmp = icmp ult i16* %incdec.ptr, %add.ptr
+  br i1 %cmp, label %while.body, label %while.end.loopexit
+
+while.end.loopexit:                               ; preds = %while.body
+  %add.lcssa = phi i64 [ %add, %while.body ]
+  br label %while.end
+
+while.end:                                        ; preds = %while.end.loopexit, %entry
+  %count.0.lcssa = phi i64 [ 0, %entry ], [ %add.lcssa, %while.end.loopexit ]
+  ret i64 %count.0.lcssa
+
+; CHECK-LABEL: foo4
+; CHECK: load <2 x i16>
+; CHECK: icmp slt <2 x i16>
+}
+