frasercrmck added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCV.td:182
+def FeatureExtZve32x
+ : SubtargetFeature<"experimental-zve32x", "HasStdExtZve32x", "true",
----------------
craig.topper wrote:
> frasercrmck wrote:
> > Do we need to define distinct `SubtargetFeature`s for each of these
> > extensions or could they be broken down into a single `MaxEEW` feature (32
> > or 64) in conjunction with the pre-existing F/D features. This seems like
> > it's more complicated than it needs to be.
> I don't think it is quite that simple. Couldn't you have a scalar D and have
> zve64f vector?
Yes, that's fair. Though I still think we can create a more intuitive system of
`Predicate`s to handle the TableGen aspects, as you've begun to do in D112496.
================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:141
+ // either v or zve* suppaort v instructions
+ bool hasStdExtV() const { return HasStdExtV || HasStdExtZve32x; }
+ bool hasStdExtZve32x() const { return HasStdExtZve32x; }
----------------
craig.topper wrote:
> frasercrmck wrote:
> > Is this correct? I thought we'd keep `hasStdExtV` as being the
> > single-letter V extension, and Zve32x isn't that.
> I just put up D112496 to stop using hasStdExtV everywhere.
Ah right now I see what this was trying to do. I think your patch helps things,
thanks.
================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-bitcast.ll:158
; CHECK-NEXT: vmv.x.s a0, v8
+; CHECK-NEXT: lui a1, 1048560
+; CHECK-NEXT: or a0, a0, a1
----------------
craig.topper wrote:
> frasercrmck wrote:
> > What's going on here, do you know?
> I believe this is coming from the nan-boxing for f16 in
> RISCVTargetLowering::splitValueIntoRegisterParts. The addition of +f must
> have changed PartVT from i32/i64 to f32. Even though we're using i32 for the
> return due to ABI.
Ah, interesting. I can't tell if that's a bug fix, i.e., if it's invalid to
compile this test without `f` - though shouldn't we pass `experimental-zfh` by
that same logic? Regardless, maybe we could split this off and pre-commit it?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112408/new/
https://reviews.llvm.org/D112408
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
