This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Bump vector crypto to v1.0 RC2
ClosedPublic

Authored by 4vtomat on Aug 16 2023, 3:16 AM.

Diff Detail

Event Timeline

4vtomat created this revision.Aug 16 2023, 3:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2023, 3:16 AM
4vtomat requested review of this revision.Aug 16 2023, 3:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2023, 3:16 AM
4vtomat updated this revision to Diff 550691.Aug 16 2023, 3:20 AM

Update llvm/docs/RISCVUsage.rst.

This revision is now accepted and ready to land.Aug 16 2023, 9:58 AM
craig.topper added inline comments.Aug 16 2023, 10:00 AM
llvm/test/MC/RISCV/rvv/zvbb.s
1

Split this file into zvkb.s and zvbb.s?

craig.topper requested changes to this revision.EditedAug 16 2023, 10:05 AM

Do we need to update anything in clang? Like riscv_vector.td? Also all the intrinsic test command lines in both clang and llvm

This revision now requires changes to proceed.Aug 16 2023, 10:05 AM

Should Zvkb and Zvbb be in this list?

void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics(                          
    ArrayRef<RVVIntrinsicRecord> Recs, IntrinsicKind K) {                        
  const TargetInfo &TI = Context.getTargetInfo();                                
  static const std::pair<const char *, uint8_t> FeatureCheckList[] = {           
      {"64bit", RVV_REQ_RV64},                                                   
      {"xsfvcp", RVV_REQ_Xsfvcp}};

Should Zvkb and Zvbb be in this list?

void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics(                          
    ArrayRef<RVVIntrinsicRecord> Recs, IntrinsicKind K) {                        
  const TargetInfo &TI = Context.getTargetInfo();                                
  static const std::pair<const char *, uint8_t> FeatureCheckList[] = {           
      {"64bit", RVV_REQ_RV64},                                                   
      {"xsfvcp", RVV_REQ_Xsfvcp}};

If we add Zvkb and Zvbb to the list, we should also add other vector crypto extensions to the list right?

Should Zvkb and Zvbb be in this list?

void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics(                          
    ArrayRef<RVVIntrinsicRecord> Recs, IntrinsicKind K) {                        
  const TargetInfo &TI = Context.getTargetInfo();                                
  static const std::pair<const char *, uint8_t> FeatureCheckList[] = {           
      {"64bit", RVV_REQ_RV64},                                                   
      {"xsfvcp", RVV_REQ_Xsfvcp}};

If we add Zvkb and Zvbb to the list, we should also add other vector crypto extensions to the list right?

Yes. Should be a different patch I suppose

Should Zvkb and Zvbb be in this list?

void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics(                          
    ArrayRef<RVVIntrinsicRecord> Recs, IntrinsicKind K) {                        
  const TargetInfo &TI = Context.getTargetInfo();                                
  static const std::pair<const char *, uint8_t> FeatureCheckList[] = {           
      {"64bit", RVV_REQ_RV64},                                                   
      {"xsfvcp", RVV_REQ_Xsfvcp}};

If we add Zvkb and Zvbb to the list, we should also add other vector crypto extensions to the list right?

Yes. Should be a different patch I suppose

OK, and I will have another patch to update the list for all vector crypto extensions~

4vtomat updated this revision to Diff 551364.Aug 17 2023, 8:53 PM

Address Craig's comments.

Herald added a project: Restricted Project. · View Herald TranscriptAug 17 2023, 8:53 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
This revision is now accepted and ready to land.Aug 17 2023, 9:00 PM
This revision was landed with ongoing or failed builds.Aug 17 2023, 9:20 PM
This revision was automatically updated to reflect the committed changes.