Skip to content

Conversation

@4vtomat
Copy link
Member

@4vtomat 4vtomat commented Nov 18, 2024

The spec: https://github.com/riscv-non-isa/rvv-intrinsic-doc/releases/tag/v1.0.0-rc4
Also remove __riscv_v_intrinsic_overloading since it's no longer in
spec, the overloading intrinsics should be also enabled when RVV
intrinsics are defined.

The spec: https://github.com/riscv-non-isa/rvv-intrinsic-doc/releases/tag/v1.0.0-rc4
Also remove __riscv_v_intrinsic_overloading since it's no longer in
spec, the overloading intrinsics should be also enabled when RVV
intrinsics are defined.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 18, 2024

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-clang

Author: Brandon Wu (4vtomat)

Changes

The spec: https://github.com/riscv-non-isa/rvv-intrinsic-doc/releases/tag/v1.0.0-rc4
Also remove __riscv_v_intrinsic_overloading since it's no longer in
spec, the overloading intrinsics should be also enabled when RVV
intrinsics are defined.


Full diff: https://github.com/llvm/llvm-project/pull/116597.diff

3 Files Affected:

  • (modified) clang/lib/Basic/Targets/RISCV.cpp (+2-2)
  • (modified) clang/test/Preprocessor/riscv-target-features.c (+6-6)
  • (modified) clang/utils/TableGen/RISCVVEmitter.cpp (-2)
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index eaaba7642bd7b2..628fdfd404698d 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -214,8 +214,8 @@ void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
 
   if (ISAInfo->hasExtension("zve32x")) {
     Builder.defineMacro("__riscv_vector");
-    // Currently we support the v0.12 RISC-V V intrinsics.
-    Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(0, 12)));
+    // Currently we support the v1.0 RISC-V V intrinsics.
+    Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(1, 0)));
   }
 
   auto VScale = getVScaleRange(Opts);
diff --git a/clang/test/Preprocessor/riscv-target-features.c b/clang/test/Preprocessor/riscv-target-features.c
index 7e8d1fa2448b80..13125d749c5fab 100644
--- a/clang/test/Preprocessor/riscv-target-features.c
+++ b/clang/test/Preprocessor/riscv-target-features.c
@@ -536,7 +536,7 @@
 // CHECK-V-EXT: __riscv_v 1000000{{$}}
 // CHECK-V-EXT: __riscv_v_elen 64
 // CHECK-V-EXT: __riscv_v_elen_fp 64
-// CHECK-V-EXT: __riscv_v_intrinsic 12000{{$}}
+// CHECK-V-EXT: __riscv_v_intrinsic 1000000{{$}}
 // CHECK-V-EXT: __riscv_v_min_vlen 128
 // CHECK-V-EXT: __riscv_vector 1
 
@@ -1244,7 +1244,7 @@
 // RUN:   -o - | FileCheck --check-prefix=CHECK-ZVE32F-EXT %s
 // CHECK-ZVE32F-EXT: __riscv_v_elen 32
 // CHECK-ZVE32F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 12000{{$}}
+// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 1000000{{$}}
 // CHECK-ZVE32F-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32F-EXT: __riscv_vector 1
 // CHECK-ZVE32F-EXT: __riscv_zve32f 1000000{{$}}
@@ -1258,7 +1258,7 @@
 // RUN:   -o - | FileCheck --check-prefix=CHECK-ZVE32X-EXT %s
 // CHECK-ZVE32X-EXT: __riscv_v_elen 32
 // CHECK-ZVE32X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 12000{{$}}
+// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 1000000{{$}}
 // CHECK-ZVE32X-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32X-EXT: __riscv_vector 1
 // CHECK-ZVE32X-EXT: __riscv_zve32x 1000000{{$}}
@@ -1271,7 +1271,7 @@
 // RUN:   -o - | FileCheck --check-prefix=CHECK-ZVE64D-EXT %s
 // CHECK-ZVE64D-EXT: __riscv_v_elen 64
 // CHECK-ZVE64D-EXT: __riscv_v_elen_fp 64
-// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 12000{{$}}
+// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 1000000{{$}}
 // CHECK-ZVE64D-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64D-EXT: __riscv_vector 1
 // CHECK-ZVE64D-EXT: __riscv_zve32f 1000000{{$}}
@@ -1288,7 +1288,7 @@
 // RUN:   -o - | FileCheck --check-prefix=CHECK-ZVE64F-EXT %s
 // CHECK-ZVE64F-EXT: __riscv_v_elen 64
 // CHECK-ZVE64F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 12000{{$}}
+// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 1000000{{$}}
 // CHECK-ZVE64F-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64F-EXT: __riscv_vector 1
 // CHECK-ZVE64F-EXT: __riscv_zve32f 1000000{{$}}
@@ -1304,7 +1304,7 @@
 // RUN:   -o - | FileCheck --check-prefix=CHECK-ZVE64X-EXT %s
 // CHECK-ZVE64X-EXT: __riscv_v_elen 64
 // CHECK-ZVE64X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 12000{{$}}
+// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 1000000{{$}}
 // CHECK-ZVE64X-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64X-EXT: __riscv_vector 1
 // CHECK-ZVE64X-EXT: __riscv_zve32x 1000000{{$}}
diff --git a/clang/utils/TableGen/RISCVVEmitter.cpp b/clang/utils/TableGen/RISCVVEmitter.cpp
index 68d7831c117458..acba1a31912816 100644
--- a/clang/utils/TableGen/RISCVVEmitter.cpp
+++ b/clang/utils/TableGen/RISCVVEmitter.cpp
@@ -488,8 +488,6 @@ void RVVEmitter::createHeader(raw_ostream &OS) {
     }
   }
 
-  OS << "#define __riscv_v_intrinsic_overloading 1\n";
-
   OS << "\n#ifdef __cplusplus\n";
   OS << "}\n";
   OS << "#endif // __cplusplus\n";

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add ReleaseNote to Clang?

@4vtomat
Copy link
Member Author

4vtomat commented Nov 19, 2024

Add ReleaseNote to Clang?

Updated!

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@rofirrim rofirrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @4vtomat !

@camel-cdr
Copy link

Considering the define is guarded behind if (ISAInfo->hasExtension("zve32x")), this doesn't seem to implement: riscv-non-isa/rvv-intrinsic-doc#382

@jrtc27
Copy link
Collaborator

jrtc27 commented Nov 19, 2024

Considering the define is guarded behind if (ISAInfo->hasExtension("zve32x")), this doesn't seem to implement: riscv-non-isa/rvv-intrinsic-doc#382

Presumably that (rather terse) sentence in the specification is that even, say, RV64GC can check the macro and include riscv_vector.h, so that it can then do __attribute__((__target__(...))) and use it in certain functions?

@topperc
Copy link
Collaborator

topperc commented Nov 19, 2024

Considering the define is guarded behind if (ISAInfo->hasExtension("zve32x")), this doesn't seem to implement: riscv-non-isa/rvv-intrinsic-doc#382

Presumably that (rather terse) sentence in the specification is that even, say, RV64GC can check the macro and include riscv_vector.h, so that it can then do __attribute__((__target__(...))) and use it in certain functions?

This patch was posted before riscv-non-isa/rvv-intrinsic-doc#382 was merged into the spec. I expect there will be a follow up patch for this.

@camel-cdr
Copy link

Presumably that (rather terse) sentence in the specification is that even, say, RV64GC can check the macro and include riscv_vector.h, so that it can then do __attribute__((__target__(...))) and use it in certain functions?

Yes, thats the idea, otherwise it would be impossible to portably use RVV intrinsics in target attribute functions, without matching specific compiler versions.

I expect there will be a follow up patch for this.

👍

@4vtomat
Copy link
Member Author

4vtomat commented Nov 22, 2024

Considering the define is guarded behind if (ISAInfo->hasExtension("zve32x")), this doesn't seem to implement: riscv-non-isa/rvv-intrinsic-doc#382

I'll post another patch for this, thanks!

@4vtomat 4vtomat merged commit 05b3d26 into llvm:main Nov 22, 2024
9 checks passed
@4vtomat 4vtomat deleted the bump_rvv_intrinsics branch November 22, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants