[RISCV] Put Large Code Model Constant Pools in .text #151393

lenary · 2025-07-30T20:49:44Z

These are required to be close to code, unlike .rodata which was being used before.

Fixes: #145080

This is requried to be close to code, unlike `.rodata` which was being used before. Fixes: llvm#145080

llvmbot · 2025-07-30T20:50:18Z

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-backend-spir-v

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

Author: Sam Elliott (lenary)

Changes

These are required to be close to code, unlike .rodata which was being used before.

Fixes: #145080

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

2 Files Affected:

(modified) llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp (+9)
(added) llvm/test/CodeGen/RISCV/large-codemodel-sections.ll (+102)

diff --git a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp
index bc90cf8f53aba..be160817c4ca7 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp
@@ -161,6 +161,15 @@ bool RISCVELFTargetObjectFile::isConstantInSmallSection(
 MCSection *RISCVELFTargetObjectFile::getSectionForConstant(
     const DataLayout &DL, SectionKind Kind, const Constant *C,
     Align &Alignment) const {
+
+  // The large code model has to put constant pools close to the program, so we
+  // put them in the .text section. Large code model doesn't support PIC, so
+  // there should be no dynamic relocations that would require `.data.rel.ro`
+  // (which could be too far away anyway).
+  if (TM->getCodeModel() == CodeModel::Large) {
+    return TextSection;
+  }
+
   if (C && isConstantInSmallSection(DL, C)) {
     if (Kind.isMergeableConst4())
       return SmallROData4Section;
diff --git a/llvm/test/CodeGen/RISCV/large-codemodel-sections.ll b/llvm/test/CodeGen/RISCV/large-codemodel-sections.ll
new file mode 100644
index 0000000000000..d5cb80ebbf74e
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/large-codemodel-sections.ll
@@ -0,0 +1,102 @@
+; RUN: llc -mtriple=riscv64 -mattr=+f,+zfh -target-abi=lp64f -code-model=large -verify-machineinstrs < %s \
+; RUN:   -filetype=obj -o - | llvm-objdump -htr - \
+; RUN:   | FileCheck %s -check-prefix=RV64I
+; RUN: llc -mtriple=riscv64 -mattr=+zfinx,+zhinx -target-abi=lp64 -code-model=large -verify-machineinstrs < %s \
+; RUN:   -filetype=obj -o - | llvm-objdump -htr - \
+; RUN:   | FileCheck %s -check-prefix=RV64I
+
+
+;; This tests that we are lowering large code model constants into `.text`
+;; constant pools, so that accessing them is close to `.text`, rather than
+;; far away in `.data`. The other choices are `.rodata` and `.data.rel.ro`,
+;; both of which may not be close enough to `.text` to be referenced.
+
+; RV64I-LABEL: Sections:
+; RV64I: .text 000000b4
+; RV64I: .rela.text 00000060
+; RV64I: .bss 00000010
+; RV64I: .data 00000002
+
+; RV64I-LABEL: SYMBOL TABLE:
+; RV64I: g O .bss 0000000000000004 G
+; RV64I: g F .text {{[0-9a-f]+}} lower_global
+; RV64I: g O .bss 0000000000000008 addr
+; RV64I: g F .text {{[0-9a-f]+}} lower_blockaddress
+; RV64I: g F .text {{[0-9a-f]+}} lower_blockaddress_displ
+; RV64I: g F .text {{[0-9a-f]+}} lower_constantpool
+; RV64I: w *UND* 0000000000000000 W
+; RV64I: g F .text {{[0-9a-f]+}} lower_extern_weak
+; RV64I: g O .data 0000000000000002 X
+; RV64I: g F .text {{[0-9a-f]+}} lower_global_half
+
+; RV64I-LABEL: RELOCATION RECORDS FOR [.text]:
+; RV64I: R_RISCV_64 G
+; RV64I: R_RISCV_64 addr
+; RV64I: R_RISCV_64 W
+; RV64I: R_RISCV_64 X
+
+
+; Check lowering of globals
+@G = global i32 0
+define i32 @lower_global(i32 %a) nounwind {
+  %1 = load volatile i32, ptr @G
+  ret i32 %1
+}
+
+; Check lowering of blockaddresses
+@addr = global ptr null
+define void @lower_blockaddress() nounwind {
+  store volatile ptr blockaddress(@lower_blockaddress, %block), ptr @addr
+  ret void
+
+block:
+  unreachable
+}
+
+; Check lowering of blockaddress that forces a displacement to be added
+define signext i32 @lower_blockaddress_displ(i32 signext %w) nounwind {
+entry:
+  %x = alloca ptr, align 8
+  store ptr blockaddress(@lower_blockaddress_displ, %test_block), ptr %x, align 8
+  %cmp = icmp sgt i32 %w, 100
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:
+  %addr = load ptr, ptr %x, align 8
+  br label %indirectgoto
+
+if.end:
+  br label %return
+
+test_block:
+  br label %return
+
+return:
+  %retval = phi i32 [ 3, %if.end ], [ 4, %test_block ]
+  ret i32 %retval
+
+indirectgoto:
+  indirectbr ptr %addr, [ label %test_block ]
+}
+
+; Check lowering of constantpools
+define float @lower_constantpool(float %a) nounwind {
+  %1 = fadd float %a, 1.000244140625
+  ret float %1
+}
+
+; Check lowering of extern_weaks
+@W = extern_weak global i32
+
+define i32 @lower_extern_weak(i32 %a) nounwind {
+  %1 = load volatile i32, ptr @W
+  ret i32 %1
+}
+
+@X = global half 1.5
+
+define half @lower_global_half(half %a) nounwind {
+  %b = load half, ptr @X
+  %1 = fadd half %a, %b
+  ret half %1
+}

lenary · 2025-07-30T20:52:17Z

I'm not sure how "good" this test is. Update LLC Test Checks wasn't helpful because it would end at the end of a function and not show the constant pools themselves. Maybe objdump -dr might be better than looking at the section and symbol tables.

llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp

llvm/test/CodeGen/RISCV/large-codemodel-sections.ll

lenary · 2025-07-30T23:48:39Z

llvm/test/CodeGen/RISCV/large-codemodel-sections.ll

+; RV64I-LABEL: RELOCATION RECORDS FOR [.text]:
+; RV64I: R_RISCV_64 G
+; RV64I: R_RISCV_64 addr
+; RV64I: R_RISCV_64 W
+; RV64I: R_RISCV_64 X


These are the relocation records for the symbols, showing they're in the .text section.

Maybe these checks aren't resilient enough and I should be using -NEXT: to help that.

I can see that the Symbol and Section tables are a bit useless for this test.

That said, I think that we do want to check that the float 1.000244140625 is also put into .text, which won't have a relocation, which is currently difficult.

topperc

LGTM

tclin914

LGTM

efriedma-quic · 2025-07-31T01:30:06Z

llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp

+  // there should be no dynamic relocations that would require `.data.rel.ro`
+  // (which could be too far away anyway).
+  if (TM->getCodeModel() == CodeModel::Large)
+    return TextSection;


I don't see any reason to expect that .text is with +-2GB of a function in an arbitrary section. The large code model has no limit on the amount of code you're allowed to include in a program. If you need to ensure distance, you need to emit the constant pool into the same section as the function.

I have addressed this, I'm not sure very well - I added a const Function * parameter because we don't have the function's section when emitConstantPool is called. This allows us to call into the section resolution logic for the function itself.

…l-large-model

efriedma-quic

I was sort of surprised to see target-independent changes here, but I guess the only target I can think of that embeds constant pools in the same section is 32-bit Arm, which doesn't use normal constant pool lowering.

efriedma-quic · 2025-07-31T16:51:58Z

llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp

+  // there should be no dynamic relocations that would require `.data.rel.ro`
+  // (which could be too far away anyway).
+  if (TM->getCodeModel() == CodeModel::Large) {
+    if (F)


Is it actually possible for F to be null here? It looks like every caller passes in a Function.

It will never make it from the call-site to here, but the X86 Asm Printer makes a call without a function, which is why I chose a pointer rather than a reference, and pointers need to be checked for null-ness. It won't be hit, but I'd prefer it to cope with nullptr in case there are more call-sites in the future.

Especially as it is not the case that every Constant has a Function, at the conceptual level in LLVM

Generally, if you actually need a constant, either you're emitting a function, or you're emitting a global. Otherwise, what's actually going to use the constant?

But I guess it's not a big deal either way.

efriedma-quic

LGTM

efriedma-quic · 2025-07-31T18:25:28Z

llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp

+  // there should be no dynamic relocations that would require `.data.rel.ro`
+  // (which could be too far away anyway).
+  if (TM->getCodeModel() == CodeModel::Large) {
+    if (F)


Generally, if you actually need a constant, either you're emitting a function, or you're emitting a global. Otherwise, what's actually going to use the constant?

But I guess it's not a big deal either way.

shaochung · 2025-08-01T07:30:03Z

The following code is generated by GCC but I think LLVM has the same problem. After this patch, in large model, the assembler can not assume the text section only has 32-bit instructions without zca extension. The text section can own 2 byte data now. This wrong assumption will lead the insufficient nop bytes insertion for code align as shown in this case. Only 4 byte nop is inserted but linker requires 6 byte for relaxation.

https://godbolt.org/z/dG8csEjEr

Option:-O0 -march=rv64g_zfh -mcmodel=large

C code

#include<stdlib.h>

_Float16 *foo (_Float16 a) {
  _Float16 b = 1.0f;
  _Float16 *m= malloc(2);
  *m = a+b;
  return m;
}

Assembly

 .align 1
.LC0:
  .half 15360
  .align 3  //  R_RISCV_ALIGN *ABS*+0x4 is not enough 
.LC1:
  .dword malloc
  .align 2
  .globl foo
  .type foo, @function
foo:
   .....

lenary · 2025-08-01T18:32:19Z

@shaochung acknowledged. I'll work out what we can do about this, maybe a realign up to IALIGN after all the constants? I'm not sure.

[RISCV] Put Large Code Model Constant Pools in .text

2a58a45

This is requried to be close to code, unlike `.rodata` which was being used before. Fixes: llvm#145080

lenary requested review from jrtc27, kito-cheng, tclin914 and topperc July 30, 2025 20:49

llvmbot added the backend:RISC-V label Jul 30, 2025

topperc reviewed Jul 30, 2025

View reviewed changes

llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp Show resolved Hide resolved

topperc reviewed Jul 30, 2025

View reviewed changes

llvm/test/CodeGen/RISCV/large-codemodel-sections.ll Outdated Show resolved Hide resolved

Braces

f74d1cb

lenary commented Jul 30, 2025

View reviewed changes

Improve test

82debbb

topperc approved these changes Jul 31, 2025

View reviewed changes

tclin914 approved these changes Jul 31, 2025

View reviewed changes

efriedma-quic reviewed Jul 31, 2025

View reviewed changes

lenary added 2 commits July 30, 2025 20:28

Ensure we use the function's section if possible

71c1056

Merge remote-tracking branch 'origin/main' into pr/riscv-constant-poo…

d4391a3

…l-large-model

llvmbot added backend:MIPS backend:X86 llvm:codegen backend:SPIR-V backend:NVPTX backend:Lanai labels Jul 31, 2025

clang-format

73c32ba

efriedma-quic reviewed Jul 31, 2025

View reviewed changes

efriedma-quic approved these changes Jul 31, 2025

View reviewed changes

[RISCV] Put Large Code Model Constant Pools in .text #151393

Are you sure you want to change the base?

[RISCV] Put Large Code Model Constant Pools in .text #151393

Uh oh!

Conversation

lenary commented Jul 30, 2025

Uh oh!

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

Uh oh!

Uh oh!

lenary commented Jul 30, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

tclin914 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shaochung commented Aug 1, 2025

Uh oh!

lenary commented Aug 1, 2025

Uh oh!

Uh oh!

llvmbot commented Jul 30, 2025 •

edited

Loading