Jim marked an inline comment as done.
Jim added inline comments.

================
Comment at: llvm/lib/Target/RISCV/RISCV.td:186-188
+                       [FeatureExtZpsfoperand,
+                        FeatureExtZpn,
+                        FeatureExtZprvsfextra]>;
----------------
jrtc27 wrote:
> Jim wrote:
> > jrtc27 wrote:
> > > These aren't correct? RV64 doesn't have Zpsfoperand and RV32 doesn't have 
> > > Zprvsfextra.
> > RV64 has Zpsfoperand extension that just has normal GPRs as operands (RV32 
> > has even/odd paired-register operand).
> > 
> > RV32 doesn't have Zprvsfextra. All of instruction in Zprvsfextra are 
> > defined in Predicates = [HasStdExtZprvsfextra, IsRV64].
> > 
> > If P is enabled, it means Zpn+Zpsfoperand enabled on RV32, and 
> > Zpn+Zpsfoperand+Zprvsfextra enabled on RV64.
> My concern is that the internal inflexibility is going to leak out of LLVM, 
> such as into the .riscv.attributes section, and thus produce broken binaries 
> because they claim they need Zprvsfextra on RV32, which is an invalid 
> combination.
> 
> And if Zpsfoperand exists for RV64 then it shouldn't be called 
> `Paired-register operand 'P' Instructions`, because the operands are only 
> paired on RV32.
I upload a new patch https://reviews.llvm.org/D108189 to support P extension 's 
arch string.
If rv32izprvsfextra (Zprvsfextra on RV32) accepted from *.s arch attribute, it 
emits error message to forbid this invalid combination.
Before emitting _zprvsfextra string, it would check RV64 is enabled.

If llc is given -mattr=+experimental-zprvsfextra on RV32, it would be ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95588

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to