khchen marked an inline comment as done.
khchen added inline comments.
================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:164
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+ {"rocket-rv32"}};
----------------
lenary wrote:
> khchen wrote:
> > lenary wrote:
> > > Is there not a tablegen'd implementation of these based on
> > > https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/RISCV/RISCV.td#L96-L99
> > > (which will include `rocket-rv32` and `rocket-rv64` when those two
> > > schedules are landed)?
> > you are right, if generic-cpu uses rocket chip scheduler, it's okay to
> > abandon this patch.
> >
> >
> No, that's not quite what I mean. When we add the rocket schedules, there
> will be additional `ProcessorModel` entries for the rocket chips, in addition
> to the current generic models.
>
> The point in these `ProcessorModel` entries is if a user passes
> `-mcpu=generic-rv64`, then `[Feature64Bit, FeatureRVCHints]` will get turned
> on, because they chose a specific cpu. This is different to validating that
> the correct features are enabled in order to choose a cpu, which seems the
> correct way around.
>
> Then instead of checking against hard-coded lists, you use use
> `MCSubtargetInfo::isCPUStringValid(StringRef)`, which uses the info from the
> `ProcessorModel` tablegen entries.
@lenary
I see no backend uses the info (ex. RISCVSubTypeKV?) from tablegen entries,
I saw different targets use different way to impl.
`TargetInfo::isValidCPUName`.
for example:
x86 uses [[
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/X86Target.def#L9
| clang/Basic/X86Target.def]] to record (hard-codeed) valid cpu enum and
string,
arm/aarch64 uses [[
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Support/ARMTargetParser.def#L184
| llvm/Support/ARMTargetParser.def]],
and mips just hard code the valid cpu string in [[
https://github.com/llvm/llvm-project/blob/master/clang/lib/Basic/Targets/Mips.cpp#L47
| clang/lib/Basic/Targets/Mips.cpp]].
They don't use backend to check valid cpu string, so maybe this patch is
workable.
But I wonder if you are asking this patch should implement the checking for
invalid combination like `-mcpu=generic-rv32` with `-mattr=+64bit` ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71124/new/
https://reviews.llvm.org/D71124
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
