[RISCV] Set __GCC_CONSTRUCTIVE_SIZE/__GCC_DESTRUCTIVE_SIZE to 64 #162986

wangpc-pp · 2025-10-11T09:39:50Z

These two macros were added in #89446.

But the previous values may not be reasonable for RV64 systems because most
of them have a cache line size 64B. So here we change them to 64.

llvmbot · 2025-10-11T09:40:21Z

@llvm/pr-subscribers-clang

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

Author: Pengcheng Wang (wangpc-pp)

Changes

These two macros were added in #89446.

But the values may not be reasonable for RV64 systems because most
of them have a cache line size 64B.

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

2 Files Affected:

(modified) clang/lib/Basic/Targets/RISCV.h (+8-4)
(added) clang/test/Preprocessor/init-riscv.c (+12)

diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index d8b0e64c90dd6..215f82f65d8ef 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -125,10 +125,6 @@ class RISCVTargetInfo : public TargetInfo {
   ParsedTargetAttr parseTargetAttr(StringRef Str) const override;
   llvm::APInt getFMVPriority(ArrayRef<StringRef> Features) const override;
 
-  std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
-    return std::make_pair(32, 32);
-  }
-
   bool supportsCpuSupports() const override { return getTriple().isOSLinux(); }
   bool supportsCpuIs() const override { return getTriple().isOSLinux(); }
   bool supportsCpuInit() const override { return getTriple().isOSLinux(); }
@@ -178,6 +174,10 @@ class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
     resetDataLayout("e-m:e-p:32:32-i64:64-n32-S128");
   }
 
+  std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
+    return std::make_pair(32, 32);
+  }
+
   bool setABI(const std::string &Name) override {
     if (Name == "ilp32e") {
       ABI = Name;
@@ -208,6 +208,10 @@ class LLVM_LIBRARY_VISIBILITY RISCV64TargetInfo : public RISCVTargetInfo {
     resetDataLayout("e-m:e-p:64:64-i64:64-i128:128-n32:64-S128");
   }
 
+  std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
+    return std::make_pair(64, 64);
+  }
+
   bool setABI(const std::string &Name) override {
     if (Name == "lp64e") {
       ABI = Name;
diff --git a/clang/test/Preprocessor/init-riscv.c b/clang/test/Preprocessor/init-riscv.c
new file mode 100644
index 0000000000000..36b2b5a06b8c6
--- /dev/null
+++ b/clang/test/Preprocessor/init-riscv.c
@@ -0,0 +1,12 @@
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang_cc1 -E -dM -triple=riscv32 < /dev/null | \
+// RUN:     FileCheck -match-full-lines -check-prefixes=RV32 %s
+// RUN: %clang_cc1 -E -dM -triple=riscv64 < /dev/null | \
+// RUN:     FileCheck -match-full-lines -check-prefixes=RV64 %s
+
+// RV32: #define __GCC_CONSTRUCTIVE_SIZE 32
+// RV32: #define __GCC_DESTRUCTIVE_SIZE 32
+
+// RV64: #define __GCC_CONSTRUCTIVE_SIZE 64
+// RV64: #define __GCC_DESTRUCTIVE_SIZE 64
\ No newline at end of file

wangpc-pp · 2025-10-11T09:43:47Z

We found this issue accidentally. I don't know if we should document this in psabi or elsewhere, but I think the GCC should change it as well. cc @kito-cheng

lenary · 2025-10-13T18:07:20Z

How do we deal with these in the ABI? We do not want people relying on these macros in their public interfaces, much like the (u)int_fastN_t/(u)int_leastN_t types, as Andrew notes here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116662#c3

wangpc-pp · 2025-10-14T03:44:56Z

How do we deal with these in the ABI? We do not want people relying on these macros in their public interfaces, much like the (u)int_fastN_t/(u)int_leastN_t types, as Andrew notes here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116662#c3

I don't have a clear thought on this. Quoted from itanium-cxx-abi/cxx-abi#74:

I propose to close this issue since the consensus is that these values are not stable and therefore do not need to be ABI-defined.

We may expose the CacheLineSize in RISCVTuneInfo to these macros?

lenary · 2025-10-14T06:05:38Z

I wondered if we wanted a quick note in the C api doc instead of the ABI. I do agree with itanium that they should not be treated as stable, but i think it's good to explicitly note that somewhere, rather than exposing them without any relevant guidance.

AaronBallman · 2025-10-14T11:56:45Z

I wondered if we wanted a quick note in the C api doc instead of the ABI. I do agree with itanium that they should not be treated as stable, but i think it's good to explicitly note that somewhere, rather than exposing them without any relevant guidance.

We have those docs: https://clang.llvm.org/docs/LanguageExtensions.html#gcc-destructive-size-and-gcc-constructive-size

Note: the values the macros expand to are not guaranteed to be stable. They are are affected by architectures and CPU tuning flags, can change between releases of Clang and will not match the values defined by other compilers such as GCC.

kito-cheng · 2025-10-15T09:37:22Z

GCC current settings:

Arch	CONSTRUCTIVE_SIZE	DESTRUCTIVE_SIZE	Comment
x86 & x86_64	64	64
ARM	64	64	Set to L1 cache line size if tune provided
AArch64	64	256	Set to L1 cache line size if tune provided
Default	32	32	Default to -param=l1-cache-line-size= which is 32

-param=constructive-interference-size=/-param=destructive-interference-size= can override the default setting.

lenary · 2025-10-15T20:52:39Z

I added some RISC-V specific documentation here: riscv-non-isa/riscv-c-api-doc#129

clang/lib/Basic/Targets/RISCV.h

preames · 2025-11-05T15:42:37Z

Reverse ping - I'd like to see this move forward.

wangpc-pp · 2025-11-06T02:52:28Z

Reverse ping - I'd like to see this move forward.

I don't know if we have a consensus about the value here, is it OK to set values to 64 for both RV32 and RV64?

kito-cheng · 2025-11-06T08:02:26Z

FYI: GCC plan to set that to 64 for both RV32/RV64, although we got some issue when landed so it got reverted temporally.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116662

wangpc-pp · 2025-11-06T08:15:50Z

FYI: GCC plan to set that to 64 for both RV32/RV64, although we got some issue when landed so it got reverted temporally.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116662

Thanks! Then I assume we have the consensus for both LLVM and GCC. I will change it to 64B for both RV32 and RV64.

…riscv64 These two macros were added in llvm#89446. But the values may not be reasonable for RV64 systems because most of them have a cache line size 64B.

AaronBallman

Please add a release note to clang/docs/ReleaseNotes.rst telling users that this value has changed.

My LG here is only for the code changes, not the value picked. Someone more familiar with RISCV should sign off on the appropriateness of using 64-bit everywhere before we land the changes.

preames

LGTM w/minor comment addressed.

Maybe wait a day in case @topperc has concerns?

clang/docs/ReleaseNotes.rst

clang/test/Preprocessor/init-riscv.c

wangpc-pp · 2025-11-11T03:25:31Z

Is it OK to merge? @topperc

topperc

LGTM

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 Oct 11, 2025

wangpc-pp requested a review from AaronBallman October 11, 2025 09:40

wangpc-pp requested review from asb, jrtc27, kito-cheng, lukel97, preames and topperc October 11, 2025 09:40

preames reviewed Oct 31, 2025

View reviewed changes

clang/lib/Basic/Targets/RISCV.h Outdated Show resolved Hide resolved

wangpc-pp added 3 commits November 6, 2025 16:21

[RISCV] Set __GCC_CONSTRUCTIVE_SIZE/__GCC_DESTRUCTIVE_SIZE to 64 for …

2dc5e8e

…riscv64 These two macros were added in llvm#89446. But the values may not be reasonable for RV64 systems because most of them have a cache line size 64B.

Add new line at the EOF

e0f0733

Change to 64 for RV32/RV64

d233bbd

wangpc-pp force-pushed the main-riscv-hardwareInterferenceSizes branch from 147710b to d233bbd Compare November 6, 2025 08:21

wangpc-pp changed the title ~~[RISCV] Set __GCC_CONSTRUCTIVE_SIZE/__GCC_DESTRUCTIVE_SIZE to 64 for riscv64~~ [RISCV] Set __GCC_CONSTRUCTIVE_SIZE/__GCC_DESTRUCTIVE_SIZE to 64 Nov 6, 2025

AaronBallman approved these changes Nov 6, 2025

View reviewed changes

Add ReleaseNotes

a62882a

preames approved these changes Nov 7, 2025

View reviewed changes

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved

topperc reviewed Nov 7, 2025

View reviewed changes

clang/test/Preprocessor/init-riscv.c Outdated Show resolved Hide resolved

Add clang links and remove riscv-registered-target

e1a5a16

topperc approved these changes Nov 11, 2025

View reviewed changes

wangpc-pp merged commit 957f929 into llvm:main Nov 11, 2025
11 checks passed

wangpc-pp deleted the main-riscv-hardwareInterferenceSizes branch November 11, 2025 06:16

[RISCV] Set __GCC_CONSTRUCTIVE_SIZE/__GCC_DESTRUCTIVE_SIZE to 64 #162986

[RISCV] Set __GCC_CONSTRUCTIVE_SIZE/__GCC_DESTRUCTIVE_SIZE to 64 #162986

Uh oh!

Conversation

wangpc-pp commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wangpc-pp commented Oct 11, 2025

Uh oh!

lenary commented Oct 13, 2025

Uh oh!

wangpc-pp commented Oct 14, 2025

Uh oh!

lenary commented Oct 14, 2025

Uh oh!

AaronBallman commented Oct 14, 2025

Uh oh!

kito-cheng commented Oct 15, 2025

Uh oh!

lenary commented Oct 15, 2025

Uh oh!

Uh oh!

preames commented Nov 5, 2025

Uh oh!

wangpc-pp commented Nov 6, 2025

Uh oh!

kito-cheng commented Nov 6, 2025

Uh oh!

wangpc-pp commented Nov 6, 2025

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

preames left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wangpc-pp commented Nov 11, 2025

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

wangpc-pp commented Oct 11, 2025 •

edited

Loading

llvmbot commented Oct 11, 2025 •

edited

Loading