Page MenuHomePhabricator
Differential D130311 Diff 449150 clang/lib/Basic/Targets/RISCV.cpp

Changeset View

Standalone View

View Options

clang/lib/Basic/Targets/RISCV.cpp

//===--- RISCV.cpp - Implement RISCV target feature support ---------------===// //===--- RISCV.cpp - Implement RISCV target feature support ---------------===//
Lint: Lint
Inline Actions

clang-format not found in user’s local PATH; not linting file.

Lint: Lint: clang-format not found in user’s local PATH; not linting file.
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
▲ Show 20 Lines • Show All 176 Lines • ▼ Show 20 Lines if (MinVLen) {
Builder.defineMacro("__riscv_v_min_vlen", Twine(MinVLen)); Builder.defineMacro("__riscv_v_min_vlen", Twine(MinVLen));
Builder.defineMacro("__riscv_v_elen", Twine(MaxELen)); Builder.defineMacro("__riscv_v_elen", Twine(MaxELen));
Builder.defineMacro("__riscv_v_elen_fp", Twine(MaxELenFp)); Builder.defineMacro("__riscv_v_elen_fp", Twine(MaxELenFp));
} }
if (ISAInfo->hasExtension("c")) if (ISAInfo->hasExtension("c"))
Builder.defineMacro("__riscv_compressed"); Builder.defineMacro("__riscv_compressed");
if (ISAInfo->hasExtension("zve32x")) if (ISAInfo->hasVInstructions())
Builder.defineMacro("__riscv_vector"); Builder.defineMacro("__riscv_vector");
} }
const Builtin::Info RISCVTargetInfo::BuiltinInfo[] = { const Builtin::Info RISCVTargetInfo::BuiltinInfo[] = {
#define BUILTIN(ID, TYPE, ATTRS) \ #define BUILTIN(ID, TYPE, ATTRS) \
{#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr}, {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr},
#define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) \ #define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) \
{#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE}, {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE},
▲ Show 20 Lines • Show All 75 Lines • ▼ Show 20 Lines if (!ParseResult) {
return false; return false;
} else { } else {
ISAInfo = std::move(*ParseResult); ISAInfo = std::move(*ParseResult);
} }
if (ABI.empty()) if (ABI.empty())
ABI = ISAInfo->computeDefaultABI().str(); ABI = ISAInfo->computeDefaultABI().str();
// StrictFP support for vectors is incomplete.
if (ISAInfo->hasVInstructions())
asbUnsubmitted
Not Done
ReplyInline Actions

There's also code in RISCVISAInfo.cpp that does HasVector = Exts.count("zve32x") != 0. It's probably worth adding a helper (hasVInstructions?) that encapsulates this, and use it from both places.

asb: There's also code in RISCVISAInfo.cpp that does `HasVector = Exts.count("zve32x") != 0`. It's…
reamesUnsubmitted
Not Done
ReplyInline Actions

It's not clear to me why this condition is specific to embedded vector variants. Do we have strict FP with +V? Either you need to fix a comment here, or the condition. One or the other.

reames: It's not clear to me why this condition is specific to embedded vector variants. Do we have…
kito-chengUnsubmitted
Not Done
ReplyInline Actions

I think check zve32f would be better? zve32x means we have integer vector instruction but might not have any floating point vector instructions.

kito-cheng: I think check `zve32f` would be better? `zve32x` means we have integer vector instruction but…
craig.topperAuthorUnsubmitted
Done
ReplyInline Actions

V implies Zve64d implies Zve64f implies Zve32f and Zve64x. Zve32f implies Zve32x. Zve32x is the root of the vector inheritance tree.

craig.topper: V implies Zve64d implies Zve64f implies Zve32f and Zve64x. Zve32f implies Zve32x. Zve32x is the…
reamesUnsubmitted
Not Done
ReplyInline Actions

So, I went digging. I agree that our *implementation* treats V as implying Zve64d, but I can find anything in the *specification* to that effect. The feature set seems like it might be identical between the two, but I don't see anything in the spec which requires a +V implementation to claim support for Zve64d. Do you have particular wording in mind I'm missing?

(Regardless, the fact we assume this elsewhere means this is a non-blocking comment for this review. At the very least, this isn't introducing a new problem.)

reames: So, I went digging. I agree that our *implementation* treats V as implying Zve64d, but I can…
craig.topperAuthorUnsubmitted
Done
ReplyInline Actions

We removed the implication for a brief period but Krste and Andrew disagreed. I believe this is now covered by the note at the end of https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#183-v-vector-extension-for-application-processors

"As is the case with other RISC-V extensions, it is valid to include overlapping extensions in the same ISA string. For example, RV64GCV and RV64GCV_Zve64f are both valid and equivalent ISA strings, as is RV64GCV_Zve64f_Zve32x_Zvl128b."

craig.topper: We removed the implication for a brief period but Krste and Andrew disagreed. I believe this is…
reamesUnsubmitted
Not Done
ReplyInline Actions

Er, yuck that's subtle. Not quite sure I'd read it the way you do, but your read is at least easily defensible. We can wait until someone has a concrete case where they aren't implied before figuring out if that case is disallowed per the spec. :)

reames: Er, yuck that's subtle. Not quite sure I'd read it the way you do, but your read is at least…
craig.topperAuthorUnsubmitted
Done
ReplyInline Actions

Maybe their biggest issue with the split we had was that we made them mutex and issued an error.

I'm going to add the wrapper that Alex suggested so that this is more centralized.

craig.topper: Maybe their biggest issue with the split we had was that we made them mutex and issued an error.
HasStrictFP = false;
return true; return true;
} }
bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const { bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name), return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name),
/*Is64Bit=*/false); /*Is64Bit=*/false);
} }
Show All 36 Lines