[PATCH] D98236: [RISCV] Add SiFive-VIU75 for llvm

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCV.td:281-283
+                                                    FeatureStdExtV,
+                                                    FeatureStdExtZvlsseg,
+                                                    FeatureExtZvamo,
----------------
khchen wrote:
> jrtc27 wrote:
> > I don't think we should ever enable V and related extensions here for two reasons:
> > 
> > 1. They're currently experimental, so -mcpu=sifive-viu75 bypasses -menable-experimental-extensions.
> > 
> > 2. They're still drafts that are going to see changes and we don't maintain backwards compatibility between draft revisions, so as soon as there's a new draft spec we'll have to remove these extensions from this model as the core will no longer implement the spec version we do.
> > They're currently experimental, so -mcpu=sifive-viu75 bypasses -menable-experimental-extensions.
> 
> Agree.
> I think maybe we could find a way to force users to give `-menable-experimental-extensions` in llc when they use a cpu which has experimental features, it will be acceptable for me.
> 
> or maybe the cpu naming should be `-mcput=experimental-sifive-viu75` like `-mattr=+experimental-v`?
I agree, if there's a desire to enable the unratified V extensions that's a discussion to be had with the wider LLVM community. It would be unfortunate if shipping devices ended up supporting unratified extension versions that don't match the ratified version, adding extra burden in terms of compiler support.

It may make sense for LLVM to support this (where there are users and people willing to maintain support), but we'd want an llvm-dev discussion about what kind of precedent it sets, expectations in ongoing support etc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98236/new/

https://reviews.llvm.org/D98236