[RISCV] Enable fixed-length vectorization of LoopVectorizer for RISC-V Vector

[luke957]

Summary

By implementing the method "unsigned RISCVTTIImpl::getRegisterBitWidth(bool Vector)", fixed-length vectorization is enabled when possible.
Without this method, the "#pragma clang loop" directive is needed to enable vectorization(or the cost model may inform LLVM that "Vectorization is possible but not beneficial").

[craig.topper]

Test case?

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

58

Doesn't getMinRVVVectorSizeInBits() already max with 128?

[luke957]

Add test case riscv-unroll.ll

[luke957]

In D97549#2590428, @craig.topper wrote:

Test case?

It does.

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

58

It does.

[frasercrmck]

The changes LGTM, thanks for adding a test case.

However, I think the commit title/message could be expanded upon somewhat. For example, *how* does it improve LoopVectorizer support? Since we've got both fixed-length and scalable vectorization, I think it would be helpful to clarify that this relates to fixed-length vectorization. More specifically, doesn't this change *enable* fixed-length vectorization, not just improve upon it?

[luke957]

In D97549#2596672, @frasercrmck wrote:

The changes LGTM, thanks for adding a test case.

However, I think the commit title/message could be expanded upon somewhat. For example, *how* does it improve LoopVectorizer support? Since we've got both fixed-length and scalable vectorization, I think it would be helpful to clarify that this relates to fixed-length vectorization. More specifically, doesn't this change *enable* fixed-length vectorization, not just improve upon it?

Thanks for the review and advice. I have rewritten the title and message.

[HsiangKai]

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

2

-mattr=+experimental-v should be enough.

Should we specify -riscv-v-vector-bits-max in this test case?

[luke957]

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

2

Yeah, "-riscv-v-vector-bits-max" is not needed.

[frasercrmck]

LGTM. I wouldn't say my last suggestion is a hard requirement.

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

2

We could perhaps add an extra RUN line for riscv32 just to make sure nothing silly is going on.

[luke957]

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

2

Add RUN line for riscv32.

[luke957]

In D97549#2602700, @frasercrmck wrote:

LGTM. I wouldn't say my last suggestion is a hard requirement.

Yeah, it would be better to add RUN line for riscv32.

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

   bool shouldExpandReduction(const IntrinsicInst *II) const;
   bool supportsScalableVectors() const { return ST->hasStdExtV(); }
   Optional<unsigned> getMaxVScale() const;
+
+  unsigned getRegisterBitWidth(bool Vector) const {
+    if (Vector) {
+      if (ST->hasStdExtV())
+        return ST->getMinRVVVectorSizeInBits();
+      return 0;
+    }
+    return ST->getXLen();
+  }
 };
 
 } // end namespace llvm

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

+; RUN: opt < %s -loop-vectorize -mtriple=riscv64 -mattr=+experimental-v -riscv-v-vector-bits-min=128 -S | FileCheck %s
+; RUN: opt < %s -loop-vectorize -mtriple=riscv32 -mattr=+experimental-v -riscv-v-vector-bits-min=128 -S | FileCheck %s
+
+; Function Attrs: nounwind
+define i32* @array_add(i32* noalias nocapture readonly %a, i32* noalias nocapture readonly %b, i32* %c, i32 %size) {
+;CHECK-LABEL: array_add
+;CHECK: load <4 x i32>
+;CHECK: load <4 x i32>
+;CHECK: add nsw <4 x i32>
+;CHECK: store <4 x i32>
+;CHECK: ret
+entry:
+  %cmp10 = icmp sgt i32 %size, 0
+  br i1 %cmp10, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  br label %for.body
+
+for.body:                                         ; preds = %for.body.preheader, %for.body
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %for.body.preheader ]
+  %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
+  %0 = load i32, i32* %arrayidx, align 4
+  %arrayidx2 = getelementptr inbounds i32, i32* %b, i64 %indvars.iv
+  %1 = load i32, i32* %arrayidx2, align 4
+  %add = add nsw i32 %1, %0
+  %arrayidx4 = getelementptr inbounds i32, i32* %c, i64 %indvars.iv
+  store i32 %add, i32* %arrayidx4, align 4
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %lftr.wideiv = trunc i64 %indvars.iv.next to i32
+  %exitcond = icmp eq i32 %lftr.wideiv, %size
+  br i1 %exitcond, label %for.end.loopexit, label %for.body
+
+for.end.loopexit:                                 ; preds = %for.body
+  br label %for.end
+
+for.end:                                          ; preds = %for.end.loopexit, %entry
+  ret i32* %c
+}