[RISCV] Enable interleaved vectorization for RVV

[luke957]

Summary

Enable interleaved vectorization for RVV.

[craig.topper]

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

5

Why are we not checking the generated IR?

42

Do we need the debug info?

[luke957]

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

5

Checking the generated IR is better.

42

DI is not necessary.

[luke957]

ping

[frasercrmck]

Do you still need to update the diff to address the previous comments?

[luke957]

In D101469#2766122, @frasercrmck wrote:

Do you still need to update the diff to address the previous comments?

Em, I have modified the test case according to the comments "checking the generated IR" and "debug info"(DI removed). Any other comments?

[craig.topper]

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

6

Is this just checking the induction variable increment? I'd really like to see what vector instructions it generates.

[luke957]

Rebase and update.

[luke957]

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

6

For this test case, vf and uf will be 4 and 2 respectively. So the vector instructions will repeat once in one trip, and there will be an instruction like %{{.*}} = add <4 x i32> %{{.*}}, <i32 4, i32 4, i32 4, i32 4>.

[luke957]

ping

[craig.topper]

LGTM

[craig.topper]

I just noticed that this enabled interleaving in the loop vectorizer even when the V extension isn't enabled. So we now generate interleaved scalar code in some cases. Was that intentional?

[luke957]

In D101469#2801767, @craig.topper wrote:

I just noticed that this enabled interleaving in the loop vectorizer even when the V extension isn't enabled. So we now generate interleaved scalar code in some cases. Was that intentional?

Thanks for reminding me. Sorry for my carelessness.

[luke957]

In D101469#2801793, @luke957 wrote:

In D101469#2801767, @craig.topper wrote:

I just noticed that this enabled interleaving in the loop vectorizer even when the V extension isn't enabled. So we now generate interleaved scalar code in some cases. Was that intentional?

Thanks for reminding me. Sorry for my carelessness.

Fixed in https://reviews.llvm.org/D103787

[craig.topper]

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

173

I think these are two different features.

enableInterleavedAccessVectorization is for memory accesses that are interlaved.

getMaxInterleaveFactor controls loop unrolling in the vectorizer.

Which feature were you trying to enable?

[craig.topper]

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

173

I think enableInterleavedAccessVectorization reads extra data and uses shuffles to extract the elements that are needed.

[jrtc27]

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

5

Why is this not using update_test_checks.py?

12–51

This IR is very messy, Clang-output IR does not always make for clean test cases. We don't need Function Attrs comments, we don't need press comments, many of the attributes are unnecessary, and the ones that are are best done inline. IR tests should be minimal, ideally from-scratch, but whittling Clang-produced IR down to something that could feasibly have been hand-written (or generated by a simple tool, like RVV and RVA tests) is fine.

[luke957]

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

173

Yes, bool enableInterleavedAccessVectorization() should not be added here. I'll restore the code and submit a new patch.

[luke957]

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

5

Yeah, using update_test_checks.py is better. But as said update_test_checks.py itself, update_test_checks.py is not designed to be authoritative about what constitutes a good test :)

12–51

Thanks for the review. I'll update the test case to look canonical.

[luke957]

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

5

Thanks for the comment. I'll try to update the test case using update_test_checks.py.

[Miss_Grape]

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

175

Do you need to report an error if the user passes the command "-force-vector-interleave=value" that value > 2

llvm/lib/Target/RISCV/RISCVSubtarget.h

   bool EnableSaveRestore = false;
   unsigned XLen = 32;
   MVT XLenVT = MVT::i32;
+  uint8_t MaxInterleaveFactor = 2;
   RISCVABI::ABI TargetABI = RISCVABI::ABI_Unknown;
   BitVector UserReservedRegister;
   RISCVFrameLowering FrameLowering;

     assert(i < RISCV::NUM_TARGET_REGS && "Register out of range");
     return UserReservedRegister[i];
   }
+  unsigned getMaxInterleaveFactor() const { return MaxInterleaveFactor; }
 
 protected:
   // GlobalISel related APIs.

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

   bool isLegalMaskedScatter(Type *DataType, Align Alignment) {
     return isLegalMaskedGatherScatter(DataType, Alignment);
   }
+
+  bool enableInterleavedAccessVectorization() { return true; }
+  unsigned getMaxInterleaveFactor(unsigned VF) {
+    return ST->getMaxInterleaveFactor();
+  }
 };
 
 } // end namespace llvm

llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll

+; RUN: opt -loop-vectorize -dce -instcombine -mtriple riscv64-linux-gnu -mattr=+experimental-v \
+; RUN: -debug-only=loop-vectorize -riscv-v-vector-bits-min=128 -S < %s 2>&1 | FileCheck %s
+
+; CHECK-LABEL: foo
+; CHECK: LV: IC is 2
+; CHECK: %{{.*}} = add {{.*}}, 8
+
+; Function Attrs: nofree norecurse nosync nounwind writeonly
+define dso_local void @foo(i32 signext %n, i32* nocapture %A) local_unnamed_addr #0 {
+entry:
+  %cmp5 = icmp sgt i32 %n, 0
+  br i1 %cmp5, label %for.body.preheader, label %for.cond.cleanup
+
+for.body.preheader:                               ; preds = %entry
+  %wide.trip.count = zext i32 %n to i64
+  br label %for.body
+
+for.cond.cleanup.loopexit:                        ; preds = %for.body
+  br label %for.cond.cleanup
+
+for.cond.cleanup:                                 ; preds = %for.cond.cleanup.loopexit, %entry
+  ret void
+
+for.body:                                         ; preds = %for.body.preheader, %for.body
+  %indvars.iv = phi i64 [ 0, %for.body.preheader ], [ %indvars.iv.next, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %A, i64 %indvars.iv
+  %0 = trunc i64 %indvars.iv to i32
+  store i32 %0, i32* %arrayidx, align 4, !tbaa !4
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond.not = icmp eq i64 %indvars.iv.next, %wide.trip.count
+  br i1 %exitcond.not, label %for.cond.cleanup.loopexit, label %for.body, !llvm.loop !8
+}
+
+attributes #0 = { nofree norecurse nosync nounwind writeonly "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+a,+c,+m,+relax,-save-restore" }
+
+!llvm.module.flags = !{!0, !1, !2}
+!llvm.ident = !{!3}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 1, !"target-abi", !"lp64"}
+!2 = !{i32 1, !"SmallDataLimit", i32 8}
+!3 = !{!"clang version 13.0.0"}
+!4 = !{!5, !5, i64 0}
+!5 = !{!"int", !6, i64 0}
+!6 = !{!"omnipotent char", !7, i64 0}
+!7 = !{!"Simple C/C++ TBAA"}
+!8 = distinct !{!8, !9}
+!9 = !{!"llvm.loop.mustprogress"}