[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck created this revision.
frasercrmck added reviewers: craig.topper, rogfer01, HsiangKai, evandro,
arcbbb, khchen.
Herald added subscribers: vkmr, luismarques, apazos, sameer.abuasal, s.egerton,
Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck,
edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal,
simoncook, johnrusso, rbar, asb.
frasercrmck requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.
This patch begins the process of supporting the `vscale_range` attribute
for RVV. It implements it according to our supported v0.10 version of
the specification, as opposed to the imminent v1.0.
Most notably, this patch implements the attribute conservatively
according to the minimum and maximum values of VLEN according to the
specification. However, the backend can be given more information about
VLEN using the `-riscv-v-vector-bits-min` and `-riscv-v-vector-bits-max`
flags. This means that the API it aims to replace,
`TargetTransformInfo::getMaxVScale`, may still generate better code with
its better knowledge.
It is unclear whether we want to move those backend options up into the
frontend, whether we are able to allow the backend to infer all
information from the IR attribute, or whether we even want to do that;
that's a wider discussion.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107290
Files:
clang/lib/Basic/Targets/RISCV.cpp
clang/lib/Basic/Targets/RISCV.h
clang/test/CodeGen/riscv-vscale-range.c
Index: clang/test/CodeGen/riscv-vscale-range.c
===
--- /dev/null
+++ clang/test/CodeGen/riscv-vscale-range.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v -S
-emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: @func() #0
+// CHECK: attributes #0 = { {{.*}} vscale_range(2,1024) {{.*}} }
+void func() {}
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -111,7 +111,11 @@
DiagnosticsEngine &Diags) override;
bool hasExtIntType() const override { return true; }
+
+ Optional>
+ getVScaleRange(const LangOptions &LangOpts) const override;
};
+
class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
public:
RISCV32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -335,6 +335,20 @@
return true;
}
+Optional>
+RISCVTargetInfo::getVScaleRange(const LangOptions &LangOpts) const {
+ if (!HasV)
+return None;
+ // VLEN is defined in v0.10 to be at least 128 bits and at most 65536 bits,
+ // and vscale is VLEN/64.
+ // FIXME: v1.0 removes the minimum value.
+ // FIXME: The backend can be told about the more specific minimum/maximum
+ // VLEN but the frontend can't access this information.
+ unsigned VLENMin = 128;
+ unsigned VLENMax = 65536;
+ return std::make_pair(VLENMin / 64, VLENMax / 64);
+}
+
bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name),
/*Is64Bit=*/false);
Index: clang/test/CodeGen/riscv-vscale-range.c
===
--- /dev/null
+++ clang/test/CodeGen/riscv-vscale-range.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: @func() #0
+// CHECK: attributes #0 = { {{.*}} vscale_range(2,1024) {{.*}} }
+void func() {}
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -111,7 +111,11 @@
DiagnosticsEngine &Diags) override;
bool hasExtIntType() const override { return true; }
+
+ Optional>
+ getVScaleRange(const LangOptions &LangOpts) const override;
};
+
class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
public:
RISCV32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -335,6 +335,20 @@
return true;
}
+Optional>
+RISCVTargetInfo::getVScaleRange(const LangOptions &LangOpts) const {
+ if (!HasV)
+return None;
+ // VLEN is defined in v0.10 to be at least 128 bits and at most 65536 bits,
+ // and vscale is VLEN/64.
+ // FIXME: v1.0 removes the minimum value.
+ // FIXME: The backend can be told about the more specific m
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck updated this revision to Diff 367232.
frasercrmck added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
update usage in vein of AArch64:
- use vscale_range attribute to determine RVV vector bits min/max values
- if no attribute is present, use existing backend flags
- sanitize and pass RVV vector bits from RISCVTargetMachine through to
RISCVSubtarget
- RISCVSubtarget just stores and reports
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107290/new/
https://reviews.llvm.org/D107290
Files:
clang/lib/Basic/Targets/RISCV.cpp
clang/lib/Basic/Targets/RISCV.h
clang/test/CodeGen/riscv-vscale-range.c
llvm/lib/Target/RISCV/RISCVSubtarget.cpp
llvm/lib/Target/RISCV/RISCVSubtarget.h
llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
Index: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
===
--- llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -32,6 +32,18 @@
#include "llvm/Target/TargetOptions.h"
using namespace llvm;
+static cl::opt RVVVectorBitsMaxOpt(
+"riscv-v-vector-bits-max",
+cl::desc("Assume V extension vector registers are at most this big, "
+ "with zero meaning no maximum size is assumed."),
+cl::init(0), cl::Hidden);
+
+static cl::opt RVVVectorBitsMinOpt(
+"riscv-v-vector-bits-min",
+cl::desc("Assume V extension vector registers are at least this big, "
+ "with zero meaning no minimum size is assumed."),
+cl::init(0), cl::Hidden);
+
extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
RegisterTargetMachine X(getTheRISCV32Target());
RegisterTargetMachine Y(getTheRISCV64Target());
@@ -78,13 +90,50 @@
Attribute TuneAttr = F.getFnAttribute("tune-cpu");
Attribute FSAttr = F.getFnAttribute("target-features");
+ unsigned RVVBitsMin = 0;
+ unsigned RVVBitsMax = 0;
+ Attribute VScaleRangeAttr = F.getFnAttribute(Attribute::VScaleRange);
+ if (VScaleRangeAttr.isValid()) {
+std::tie(RVVBitsMin, RVVBitsMax) = VScaleRangeAttr.getVScaleRangeArgs();
+RVVBitsMin *= RISCV::RVVBitsPerBlock;
+RVVBitsMax *= RISCV::RVVBitsPerBlock;
+ } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;
+ }
+
+ assert(RVVBitsMin % 128 == 0 &&
+ "RVV requires vector length in multiples of 128!");
+ assert(RVVBitsMax % 128 == 0 &&
+ "RVV requires vector length in multiples of 128!");
+ assert(RVVBitsMax <= 65536 &&
+ "RVV vector size must be no larger than 65536!");
+ assert((RVVBitsMax >= RVVBitsMin || RVVBitsMax == 0) &&
+ "Minimum RVV vector size should not be larger than its maximum!");
+
+ // Sanitize user input in case of no asserts.
+ if (RVVBitsMax != 0)
+RVVBitsMin = std::min(RVVBitsMin, RVVBitsMax);
+ RVVBitsMin =
+ PowerOf2Floor((RVVBitsMin < 128 || RVVBitsMin > 65536) ? 0 : RVVBitsMin);
+
+ RVVBitsMax = std::max(RVVBitsMin, RVVBitsMax);
+ RVVBitsMax =
+ PowerOf2Floor((RVVBitsMax < 128 || RVVBitsMax > 65536) ? 0 : RVVBitsMax);
+
std::string CPU =
CPUAttr.isValid() ? CPUAttr.getValueAsString().str() : TargetCPU;
std::string TuneCPU =
TuneAttr.isValid() ? TuneAttr.getValueAsString().str() : CPU;
std::string FS =
FSAttr.isValid() ? FSAttr.getValueAsString().str() : TargetFS;
+
std::string Key = CPU + TuneCPU + FS;
+ Key += "RVVMin";
+ Key += std::to_string(RVVBitsMin);
+ Key += "RVVMax";
+ Key += std::to_string(RVVBitsMax);
+
auto &I = SubtargetMap[Key];
if (!I) {
// This needs to be done before we create a new subtarget since any
@@ -101,7 +150,8 @@
}
ABIName = ModuleTargetABI->getString();
}
-I = std::make_unique(TargetTriple, CPU, TuneCPU, FS, ABIName, *this);
+I = std::make_unique(
+TargetTriple, CPU, TuneCPU, FS, ABIName, RVVBitsMin, RVVBitsMax, *this);
}
return I.get();
}
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -62,6 +62,8 @@
bool EnableSaveRestore = false;
unsigned XLen = 32;
MVT XLenVT = MVT::i32;
+ unsigned RVVVectorBitsMin;
+ unsigned RVVVectorBitsMax;
uint8_t MaxInterleaveFactor = 2;
RISCVABI::ABI TargetABI = RISCVABI::ABI_Unknown;
BitVector UserReservedRegister;
@@ -82,7 +84,8 @@
public:
// Initializes the data members to match that of the specified triple.
RISCVSubtarget(const Triple &TT, StringRef CPU, StringRef TuneCPU,
- StringRef FS, StringRef ABIName, const TargetMachine &TM);
+ StringRef FS, StringRef ABIName, unsigned RVVVectorBitsMin,
+ unsigned RVVVectorLMULMax, const TargetMachine &TM);
// Parses features string setting specified subtarget options. The
// definition of this funct
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck added a comment. This may be as far as we can take this patch without exposing RVV vectors bit control to the user/driver and having to worry about the concerns that spring from that: linking objects compiled with different RVV vector bits options, LTO, etc. I believe that with the current state of the patch, the default, hard-coded `vscale_range` with values mandated by the spec, combined with the existing backend options for overrides, mean we're not losing any functionality. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107290/new/ https://reviews.llvm.org/D107290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck added a comment. Ah no, my mistake. This would be a drop in functionality if `getMaxVScale` is removed, since its replacement only checks the IR attribute and will not be affected by our backend flags. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107290/new/ https://reviews.llvm.org/D107290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:349 + unsigned VLENMax = 65536; + return std::make_pair(VLENMin / 64, VLENMax / 64); +} Should we move RVVBitsPerBlock to RISCVTargetParser.def? Or some other place that can be shared between lllvm/lib/Target/RISCV/ and here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107290/new/ https://reviews.llvm.org/D107290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck added inline comments.
Herald added subscribers: eopXD, VincentWu, luke957,
achieveartificialintelligence.
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:101
+ } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;
frasercrmck wrote:
> craig.topper wrote:
> > If clang always emits the attribute, are these options effectively dead for
> > clang codegen?
> Yes, that's a good point - I'd missed that. I'm not sure the best way of
> keeping that ability apart from moving the options up to clang and dealing
> with the fallout from that. Which I'm not even sure we //can// deal with yet?
>
> Unless we make the options override the attribute, though that might be its
> own can of worms.
Well we now have `zvl` which kinda solve the "min" problem at the frontend
level.
Thinking about it again, though, maybe it's not such a bad thing to have clang
emit min=, max=2^16/RVVBitsPerBlock and then allow backend codegen flags
to override that. Then the onus is clearly on the user not to do anything
wrong. We could assert if the user-provided values are clearly at odds with the
attribute?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107290/new/
https://reviews.llvm.org/D107290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
craig.topper added inline comments.
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:101
+ } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;
frasercrmck wrote:
> frasercrmck wrote:
> > craig.topper wrote:
> > > If clang always emits the attribute, are these options effectively dead
> > > for clang codegen?
> > Yes, that's a good point - I'd missed that. I'm not sure the best way of
> > keeping that ability apart from moving the options up to clang and dealing
> > with the fallout from that. Which I'm not even sure we //can// deal with
> > yet?
> >
> > Unless we make the options override the attribute, though that might be its
> > own can of worms.
> Well we now have `zvl` which kinda solve the "min" problem at the frontend
> level.
>
> Thinking about it again, though, maybe it's not such a bad thing to have
> clang emit min=, max=2^16/RVVBitsPerBlock and then allow backend codegen
> flags to override that. Then the onus is clearly on the user not to do
> anything wrong. We could assert if the user-provided values are clearly at
> odds with the attribute?
I'm fine with that. I think we should consider dropping the
riscv-v-vector-bits-min flag and just have a
-riscv-v-fixed-width-vectorization-flag until we can prove that vectorization
is robust. Bugs like D117663 make me nervous about blindly vectorizing code
right now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107290/new/
https://reviews.llvm.org/D107290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck updated this revision to Diff 402572.
frasercrmck added a comment.
Herald added a subscriber: pcwang-thead.
rebase
take minimum from zvl extensions
allow backend options to override attribute values
add extra testing
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107290/new/
https://reviews.llvm.org/D107290
Files:
clang/lib/Basic/Targets/RISCV.cpp
clang/lib/Basic/Targets/RISCV.h
clang/test/CodeGen/RISCV/riscv-vscale-range.c
llvm/include/llvm/Support/TargetParser.h
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
llvm/lib/Target/RISCV/RISCVISelLowering.h
llvm/lib/Target/RISCV/RISCVSubtarget.cpp
llvm/lib/Target/RISCV/RISCVSubtarget.h
llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vscale-range.ll
Index: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vscale-range.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vscale-range.ll
@@ -0,0 +1,167 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+v,+m -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+v,+m -verify-machineinstrs < %s | FileCheck %s
+
+define <512 x i8> @vadd_v512i8_zvl128(<512 x i8> %a, <512 x i8> %b) #0 {
+; CHECK-LABEL: vadd_v512i8_zvl128:
+; CHECK: # %bb.0:
+; CHECK-NEXT:addi sp, sp, -16
+; CHECK-NEXT:.cfi_def_cfa_offset 16
+; CHECK-NEXT:csrr a2, vlenb
+; CHECK-NEXT:li a4, 40
+; CHECK-NEXT:mul a2, a2, a4
+; CHECK-NEXT:sub sp, sp, a2
+; CHECK-NEXT:csrr a2, vlenb
+; CHECK-NEXT:li a4, 24
+; CHECK-NEXT:mul a2, a2, a4
+; CHECK-NEXT:add a2, sp, a2
+; CHECK-NEXT:addi a2, a2, 16
+; CHECK-NEXT:vs8r.v v16, (a2) # Unknown-size Folded Spill
+; CHECK-NEXT:csrr a2, vlenb
+; CHECK-NEXT:slli a2, a2, 5
+; CHECK-NEXT:add a2, sp, a2
+; CHECK-NEXT:addi a2, a2, 16
+; CHECK-NEXT:vs8r.v v8, (a2) # Unknown-size Folded Spill
+; CHECK-NEXT:li a2, 128
+; CHECK-NEXT:vsetvli zero, a2, e8, m8, ta, mu
+; CHECK-NEXT:addi a2, a3, 128
+; CHECK-NEXT:addi a4, a3, 384
+; CHECK-NEXT:vle8.v v8, (a4)
+; CHECK-NEXT:csrr a4, vlenb
+; CHECK-NEXT:slli a4, a4, 4
+; CHECK-NEXT:add a4, sp, a4
+; CHECK-NEXT:addi a4, a4, 16
+; CHECK-NEXT:vs8r.v v8, (a4) # Unknown-size Folded Spill
+; CHECK-NEXT:addi a4, a1, 128
+; CHECK-NEXT:vle8.v v8, (a1)
+; CHECK-NEXT:csrr a1, vlenb
+; CHECK-NEXT:slli a1, a1, 3
+; CHECK-NEXT:add a1, sp, a1
+; CHECK-NEXT:addi a1, a1, 16
+; CHECK-NEXT:vs8r.v v8, (a1) # Unknown-size Folded Spill
+; CHECK-NEXT:addi a1, a3, 256
+; CHECK-NEXT:vle8.v v8, (a1)
+; CHECK-NEXT:vle8.v v16, (a4)
+; CHECK-NEXT:vle8.v v24, (a2)
+; CHECK-NEXT:vle8.v v0, (a3)
+; CHECK-NEXT:addi a1, sp, 16
+; CHECK-NEXT:vs8r.v v0, (a1) # Unknown-size Folded Spill
+; CHECK-NEXT:csrr a1, vlenb
+; CHECK-NEXT:slli a1, a1, 3
+; CHECK-NEXT:add a1, sp, a1
+; CHECK-NEXT:addi a1, a1, 16
+; CHECK-NEXT:vl8re8.v v0, (a1) # Unknown-size Folded Reload
+; CHECK-NEXT:vadd.vv v8, v0, v8
+; CHECK-NEXT:csrr a1, vlenb
+; CHECK-NEXT:slli a1, a1, 3
+; CHECK-NEXT:add a1, sp, a1
+; CHECK-NEXT:addi a1, a1, 16
+; CHECK-NEXT:vs8r.v v8, (a1) # Unknown-size Folded Spill
+; CHECK-NEXT:csrr a1, vlenb
+; CHECK-NEXT:slli a1, a1, 4
+; CHECK-NEXT:add a1, sp, a1
+; CHECK-NEXT:addi a1, a1, 16
+; CHECK-NEXT:vl8re8.v v8, (a1) # Unknown-size Folded Reload
+; CHECK-NEXT:vadd.vv v16, v16, v8
+; CHECK-NEXT:csrr a1, vlenb
+; CHECK-NEXT:li a2, 24
+; CHECK-NEXT:mul a1, a1, a2
+; CHECK-NEXT:add a1, sp, a1
+; CHECK-NEXT:addi a1, a1, 16
+; CHECK-NEXT:vl8re8.v v8, (a1) # Unknown-size Folded Reload
+; CHECK-NEXT:vadd.vv v8, v8, v24
+; CHECK-NEXT:csrr a1, vlenb
+; CHECK-NEXT:slli a1, a1, 5
+; CHECK-NEXT:add a1, sp, a1
+; CHECK-NEXT:addi a1, a1, 16
+; CHECK-NEXT:vl8re8.v v24, (a1) # Unknown-size Folded Reload
+; CHECK-NEXT:addi a1, sp, 16
+; CHECK-NEXT:vl8re8.v v0, (a1) # Unknown-size Folded Reload
+; CHECK-NEXT:vadd.vv v0, v24, v0
+; CHECK-NEXT:vse8.v v0, (a0)
+; CHECK-NEXT:addi a1, a0, 384
+; CHECK-NEXT:vse8.v v16, (a1)
+; CHECK-NEXT:addi a1, a0, 256
+; CHECK-NEXT:csrr a2, vlenb
+; CHECK-NEXT:slli a2, a2, 3
+; CHECK-NEXT:add a2, sp, a2
+; CHECK-NEXT:addi a2, a2, 16
+; CHECK-NEXT:vl8re8.v v16, (a2) # Unknown-size Folded Reload
+; CHECK-NEXT:vse8.v v16, (a1)
+; CHECK-NEXT:addi a0, a0, 128
+; CHECK-NEXT:vse8.v v8, (a0)
+; CHECK-NEXT:csrr a0, vlenb
+; CHECK-NEXT:li a1, 40
+; CHECK-NEXT:mul a0, a0, a1
+; CHECK-NEXT:add sp, sp, a0
+; CHECK-NEXT:addi sp, sp, 16
+; CHECK-NEXT:ret
+ %c = add <512 x i8> %a, %b
+ ret <512 x i8> %c
+}
+
+define <512 x i8> @vadd_v512i8_z
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
kito-cheng added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:106 + assert(RVVBitsMin % 128 == 0 && + "RVV requires vector length in multiples of 128!"); + assert(RVVBitsMax % 128 == 0 && RISC-V require VLEN in power of 2, multiples of 128 is constraint for SVE :p https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#2-implementation-defined-constant-parameters Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107290/new/ https://reviews.llvm.org/D107290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck updated this revision to Diff 369416.
frasercrmck marked 2 inline comments as done.
frasercrmck added a comment.
Herald added a subscriber: dexonsmith.
- rebase
- move V VLEN bits-per-block (64), min (128), max (65536) defines into
TargetParser.h
- clean up assertions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107290/new/
https://reviews.llvm.org/D107290
Files:
clang/lib/Basic/Targets/RISCV.cpp
clang/lib/Basic/Targets/RISCV.h
clang/test/CodeGen/riscv-vscale-range.c
llvm/include/llvm/Support/TargetParser.h
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
llvm/lib/Target/RISCV/RISCVISelLowering.h
llvm/lib/Target/RISCV/RISCVSubtarget.cpp
llvm/lib/Target/RISCV/RISCVSubtarget.h
llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
Index: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
===
--- llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
+++ llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
@@ -21,6 +21,7 @@
#include "llvm/Analysis/TargetTransformInfo.h"
#include "llvm/CodeGen/BasicTTIImpl.h"
#include "llvm/IR/Function.h"
+#include "llvm/Support/TargetParser.h"
namespace llvm {
Index: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
===
--- llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -32,6 +32,18 @@
#include "llvm/Target/TargetOptions.h"
using namespace llvm;
+static cl::opt RVVVectorBitsMaxOpt(
+"riscv-v-vector-bits-max",
+cl::desc("Assume V extension vector registers are at most this big, "
+ "with zero meaning no maximum size is assumed."),
+cl::init(0), cl::Hidden);
+
+static cl::opt RVVVectorBitsMinOpt(
+"riscv-v-vector-bits-min",
+cl::desc("Assume V extension vector registers are at least this big, "
+ "with zero meaning no minimum size is assumed."),
+cl::init(0), cl::Hidden);
+
extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
RegisterTargetMachine X(getTheRISCV32Target());
RegisterTargetMachine Y(getTheRISCV64Target());
@@ -78,13 +90,58 @@
Attribute TuneAttr = F.getFnAttribute("tune-cpu");
Attribute FSAttr = F.getFnAttribute("target-features");
+ unsigned RVVBitsMin = 0;
+ unsigned RVVBitsMax = 0;
+ Attribute VScaleRangeAttr = F.getFnAttribute(Attribute::VScaleRange);
+ if (VScaleRangeAttr.isValid()) {
+std::tie(RVVBitsMin, RVVBitsMax) = VScaleRangeAttr.getVScaleRangeArgs();
+RVVBitsMin *= RISCV::RVVBitsPerBlock;
+RVVBitsMax *= RISCV::RVVBitsPerBlock;
+ } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;
+ }
+
+ assert((RVVBitsMin == 0 || isPowerOf2_32(RVVBitsMin)) &&
+ "RVV requires vector length to be a power of two!");
+ assert((RVVBitsMax == 0 || isPowerOf2_32(RVVBitsMax)) &&
+ "RVV requires vector length to be a power of two!");
+ assert((RVVBitsMin == 0 || RVVBitsMin >= RISCV::StdVVLENBitsMin) &&
+ "RVV vector size must be no smaller than the minimum allowed by the "
+ "specification!");
+ assert(RVVBitsMax <= RISCV::StdVVLENBitsMax &&
+ "RVV vector size must be no larger than the maximum allowed by the "
+ "specification!");
+ assert((RVVBitsMax == 0 || RVVBitsMax >= RVVBitsMin) &&
+ "Minimum RVV vector size should not be larger than its maximum!");
+
+ // Sanitize user input in case of no asserts.
+ if (RVVBitsMax != 0)
+RVVBitsMin = std::min(RVVBitsMin, RVVBitsMax);
+ RVVBitsMin = PowerOf2Floor((RVVBitsMin < RISCV::StdVVLENBitsMin ||
+ RVVBitsMin > RISCV::StdVVLENBitsMax)
+ ? 0
+ : RVVBitsMin);
+
+ RVVBitsMax = std::max(RVVBitsMin, RVVBitsMax);
+ RVVBitsMax = PowerOf2Floor((RVVBitsMax < RISCV::StdVVLENBitsMin ||
+ RVVBitsMax > RISCV::StdVVLENBitsMax)
+ ? 0
+ : RVVBitsMax);
+
std::string CPU =
CPUAttr.isValid() ? CPUAttr.getValueAsString().str() : TargetCPU;
std::string TuneCPU =
TuneAttr.isValid() ? TuneAttr.getValueAsString().str() : CPU;
std::string FS =
FSAttr.isValid() ? FSAttr.getValueAsString().str() : TargetFS;
+
std::string Key = CPU + TuneCPU + FS;
+ Key += "RVVMin";
+ Key += std::to_string(RVVBitsMin);
+ Key += "RVVMax";
+ Key += std::to_string(RVVBitsMax);
+
auto &I = SubtargetMap[Key];
if (!I) {
// This needs to be done before we create a new subtarget since any
@@ -101,7 +158,8 @@
}
ABIName = ModuleTargetABI->getString();
}
-I = std::make_unique(TargetTriple, CPU, TuneCPU, FS, ABIName, *this);
+I = std::make_unique(
+TargetTriple, CPU, TuneCPU, FS, ABIName, RVVBitsMin, RVVBitsMax, *this);
}
return
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:349 + unsigned VLENMax = 65536; + return std::make_pair(VLENMin / 64, VLENMax / 64); +} craig.topper wrote: > Should we move RVVBitsPerBlock to RISCVTargetParser.def? Or some other place > that can be shared between lllvm/lib/Target/RISCV/ and here? Good idea. I also added the "StdV" min/max values of `128`/`65536` in there. However, I just put them in `TargetParser.h` as putting them in the `.def` file felt a bit odd and you had to account for preprocessor logic. It still feels a little odd but I agree that sharing these values is important. Other targets have specific values in there so it's not unprecedented. It is target-adjacent data, even if it's not (currently) dependent on triples or cpus. Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:106 + assert(RVVBitsMin % 128 == 0 && + "RVV requires vector length in multiples of 128!"); + assert(RVVBitsMax % 128 == 0 && kito-cheng wrote: > RISC-V require VLEN in power of 2, multiples of 128 is constraint for SVE :p > https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#2-implementation-defined-constant-parameters Yeah to be honest I was just being cheeky/lazy here :) Since our current implementation requires `VLEN >= 128` we know that VLEN must always be a multiple of 128. But yes this isn't really the right way of coding it, even if it does the right thing. I've fixed that up now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107290/new/ https://reviews.llvm.org/D107290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
craig.topper added inline comments.
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:101
+ } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;
If clang always emits the attribute, are these options effectively dead for
clang codegen?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107290/new/
https://reviews.llvm.org/D107290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute
frasercrmck added inline comments.
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:101
+ } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;
craig.topper wrote:
> If clang always emits the attribute, are these options effectively dead for
> clang codegen?
Yes, that's a good point - I'd missed that. I'm not sure the best way of
keeping that ability apart from moving the options up to clang and dealing with
the fallout from that. Which I'm not even sure we //can// deal with yet?
Unless we make the options override the attribute, though that might be its own
can of worms.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107290/new/
https://reviews.llvm.org/D107290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
