[PATCH] D150253: [RISCV] Add Zvfhmin extension for clang.

[Jianjian Guan via Phabricator via llvm-commits]

jacquesguan marked 5 inline comments as done.
jacquesguan added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:162
+  bool hasVInstructionsF16Mininal() const {
+    return HasStdExtZvfhmin || HasStdExtZvfh;
+  }
----------------
craig.topper wrote:
> michaelmaitland wrote:
> > michaelmaitland wrote:
> > > jacquesguan wrote:
> > > > craig.topper wrote:
> > > > > Doesn't HasStdExtZvfh already imply HasStdExtZvfhmin?
> > > > The v spec doesn't metion this.
> > > I think the spec conveys this when it says `The Zvfhmin extension depends on the Zve32f extension.`
> > My mistake, that says `Zve32f`, not `Zvfh`. However, the spec does say:
> > 
> > `When the Zvfhmin extension is implemented, the vfwcvt.f.f.v and vfncvt.f.f.w instructions become defined when SEW=16` and also says `When the Zvfh extension is implemented, all instructions in Sections Vector Floating-Point Instructions.` Since `vfwcvt.f.f.v and vfncvt.f.f.w` are part of `Vector Floating-Point Instructions` section, this is how it is implied.
> It's implemented in LLVM by this patch https://reviews.llvm.org/D150016
This patch make Zvfh imply **Zfhmin ** not **Zvfhmin**. I think the relation between Zvfh and Zvfhmin is just like the relation between Zfh and Zfhmin. Now Zfh doesn't imply Zfhmin, so I keep Zvfh not imply Zvfhmin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253