craig.topper added inline comments.
================ Comment at: llvm/lib/Target/RISCV/RISCV.td:367 + [FeatureExtZca]>; // TODO: add Zicsr as another dependence +def HasStdExtZcmt : Predicate<"Subtarget->hasStdExtZcmt() && !Subtarget->hasStdExtC()">, + AssemblerPredicate<(all_of FeatureExtZcmt, (not FeatureStdExtC)), ---------------- craig.topper wrote: > reames wrote: > > Wait, Zcmt can't be enabled if C is? That seems odd... > I think Zcmt reuses encodings from the FP part of C. That's why C was split > into Zca, Zcf, and Zcd. This restriction needs to be enforced in RISCVISAInfo.cpp so that it generates a proper error message. ================ Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:1 +//===-- RISCVInstrInfoZc.td - RISC-V 'Zc' instructions -----*- tablegen -*-===// +// ---------------- reames wrote: > We have Zca in RISCVInstrInfoC.td. Not sure it makes sense to have Zcmt in > one place, and Zca in another. > > Also, did Zca change between 0.7 and 0.7.5? If so, any plans to update that > extension? I think all Zc* should be in RISCVInstrInfoC.td. Zca/Zcf/Zcd already have to be in that file. Might as well put the rest there. We should mention the Zc* extensions in the header of that file. ================ Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:386 + +def : SysReg<"jvt", 0x017>; ---------------- This needs an assembler test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133863/new/ https://reviews.llvm.org/D133863 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
