Skip to content

[RISCV] Remove R_RISCV_RVC_LUI Relocation #118714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

lenary
Copy link
Member

@lenary lenary commented Dec 4, 2024

This was removed from the ABI in riscv-non-isa/riscv-elf-psabi-doc#398. It is not emitted by LLVM, and seems to have been an internal implementation detail in binutils.

This is a follow-up to 26ec5da which removed previous binutils internal relocations when they were removed from the ABI.

The LLD implementation was not tested, presumably because no LLVM assembler supported this relocation.

This was removed from the ABI in riscv-non-isa/riscv-elf-psabi-doc#398.
It is not emitted by LLVM, and seems to have been an internal
implementation detail in binutils.

This is a follow-up to 26ec5da which removed previous binutils
internal relocations when they were removed from the ABI.
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2024

@llvm/pr-subscribers-lld

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

Author: Sam Elliott (lenary)

Changes

This was removed from the ABI in riscv-non-isa/riscv-elf-psabi-doc#398. It is not emitted by LLVM, and seems to have been an internal implementation detail in binutils.

This is a follow-up to 26ec5da which removed previous binutils internal relocations when they were removed from the ABI.

The LLD implementation was not tested, presumably because no LLVM assembler supported this relocation.


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

2 Files Affected:

  • (modified) lld/ELF/Arch/RISCV.cpp (-14)
  • (modified) llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def (-1)
diff --git a/lld/ELF/Arch/RISCV.cpp b/lld/ELF/Arch/RISCV.cpp
index e150ff26fc3b5e..36ae31be6ed2a2 100644
--- a/lld/ELF/Arch/RISCV.cpp
+++ b/lld/ELF/Arch/RISCV.cpp
@@ -267,7 +267,6 @@ RelExpr RISCV::getRelExpr(const RelType type, const Symbol &s,
   case R_RISCV_HI20:
   case R_RISCV_LO12_I:
   case R_RISCV_LO12_S:
-  case R_RISCV_RVC_LUI:
     return R_ABS;
   case R_RISCV_ADD8:
   case R_RISCV_ADD16:
@@ -373,19 +372,6 @@ void RISCV::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
     return;
   }
 
-  case R_RISCV_RVC_LUI: {
-    int64_t imm = SignExtend64(val + 0x800, bits) >> 12;
-    checkInt(ctx, loc, imm, 6, rel);
-    if (imm == 0) { // `c.lui rd, 0` is illegal, convert to `c.li rd, 0`
-      write16le(loc, (read16le(loc) & 0x0F83) | 0x4000);
-    } else {
-      uint16_t imm17 = extractBits(val + 0x800, 17, 17) << 12;
-      uint16_t imm16_12 = extractBits(val + 0x800, 16, 12) << 2;
-      write16le(loc, (read16le(loc) & 0xEF83) | imm17 | imm16_12);
-    }
-    return;
-  }
-
   case R_RISCV_JAL: {
     checkInt(ctx, loc, val, 21, rel);
     checkAlignment(ctx, loc, val, 2, rel);
diff --git a/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def b/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
index d4be34e3b37e5e..65a15c2e15621b 100644
--- a/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
+++ b/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
@@ -45,7 +45,6 @@ ELF_RELOC(R_RISCV_GOT32_PCREL,       41)
 ELF_RELOC(R_RISCV_ALIGN,             43)
 ELF_RELOC(R_RISCV_RVC_BRANCH,        44)
 ELF_RELOC(R_RISCV_RVC_JUMP,          45)
-ELF_RELOC(R_RISCV_RVC_LUI,           46)
 ELF_RELOC(R_RISCV_RELAX,             51)
 ELF_RELOC(R_RISCV_SUB6,              52)
 ELF_RELOC(R_RISCV_SET6,              53)

@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Sam Elliott (lenary)

Changes

This was removed from the ABI in riscv-non-isa/riscv-elf-psabi-doc#398. It is not emitted by LLVM, and seems to have been an internal implementation detail in binutils.

This is a follow-up to 26ec5da which removed previous binutils internal relocations when they were removed from the ABI.

The LLD implementation was not tested, presumably because no LLVM assembler supported this relocation.


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

2 Files Affected:

  • (modified) lld/ELF/Arch/RISCV.cpp (-14)
  • (modified) llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def (-1)
diff --git a/lld/ELF/Arch/RISCV.cpp b/lld/ELF/Arch/RISCV.cpp
index e150ff26fc3b5e..36ae31be6ed2a2 100644
--- a/lld/ELF/Arch/RISCV.cpp
+++ b/lld/ELF/Arch/RISCV.cpp
@@ -267,7 +267,6 @@ RelExpr RISCV::getRelExpr(const RelType type, const Symbol &s,
   case R_RISCV_HI20:
   case R_RISCV_LO12_I:
   case R_RISCV_LO12_S:
-  case R_RISCV_RVC_LUI:
     return R_ABS;
   case R_RISCV_ADD8:
   case R_RISCV_ADD16:
@@ -373,19 +372,6 @@ void RISCV::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
     return;
   }
 
-  case R_RISCV_RVC_LUI: {
-    int64_t imm = SignExtend64(val + 0x800, bits) >> 12;
-    checkInt(ctx, loc, imm, 6, rel);
-    if (imm == 0) { // `c.lui rd, 0` is illegal, convert to `c.li rd, 0`
-      write16le(loc, (read16le(loc) & 0x0F83) | 0x4000);
-    } else {
-      uint16_t imm17 = extractBits(val + 0x800, 17, 17) << 12;
-      uint16_t imm16_12 = extractBits(val + 0x800, 16, 12) << 2;
-      write16le(loc, (read16le(loc) & 0xEF83) | imm17 | imm16_12);
-    }
-    return;
-  }
-
   case R_RISCV_JAL: {
     checkInt(ctx, loc, val, 21, rel);
     checkAlignment(ctx, loc, val, 2, rel);
diff --git a/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def b/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
index d4be34e3b37e5e..65a15c2e15621b 100644
--- a/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
+++ b/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
@@ -45,7 +45,6 @@ ELF_RELOC(R_RISCV_GOT32_PCREL,       41)
 ELF_RELOC(R_RISCV_ALIGN,             43)
 ELF_RELOC(R_RISCV_RVC_BRANCH,        44)
 ELF_RELOC(R_RISCV_RVC_JUMP,          45)
-ELF_RELOC(R_RISCV_RVC_LUI,           46)
 ELF_RELOC(R_RISCV_RELAX,             51)
 ELF_RELOC(R_RISCV_SUB6,              52)
 ELF_RELOC(R_RISCV_SET6,              53)

@MaskRay
Copy link
Member

MaskRay commented Dec 5, 2024

The LLD implementation was not tested, presumably because no LLVM assembler supported this relocation.

You can edit this part of the description to say that it was not tested when added by https://reviews.llvm.org/D39322 .

(New targets should really test every relocation.)

@lenary lenary merged commit 65ced15 into llvm:main Dec 5, 2024
9 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 5, 2024

LLVM Buildbot has detected a new failure on builder clangd-ubuntu-tsan running on clangd-ubuntu-clang while building lld,llvm at step 6 "test-build-clangd-clangd-index-server-clangd-in...".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/134/builds/9794

Here is the relevant piece of the build log for the reference
Step 6 (test-build-clangd-clangd-index-server-clangd-in...) failure: test (failure)
******************** TEST 'Clangd :: target_info.test' FAILED ********************
Exit Code: 66

Command Output (stderr):
--
RUN: at line 5: rm -rf /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir && mkdir -p /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir
+ rm -rf /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir
+ mkdir -p /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir
RUN: at line 7: echo '[{"directory": "/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir", "command": "/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir/armv7-clang -x c++ the-file.cpp -v", "file": "the-file.cpp"}]' > /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir/compile_commands.json
+ echo '[{"directory": "/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir", "command": "/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir/armv7-clang -x c++ the-file.cpp -v", "file": "the-file.cpp"}]'
RUN: at line 9: sed -e "s|INPUT_DIR|/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir|g" /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/llvm-project/clang-tools-extra/clangd/test/target_info.test > /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test.1
+ sed -e 's|INPUT_DIR|/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir|g' /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/llvm-project/clang-tools-extra/clangd/test/target_info.test
RUN: at line 12: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test.1 > /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test
+ sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test.1
RUN: at line 14: clangd -lit-test < /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test 2>&1 | /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/bin/FileCheck -strict-whitespace /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test
+ clangd -lit-test
+ /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/bin/FileCheck -strict-whitespace /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test

--

********************


@lenary lenary deleted the pr/riscv-remove-reloc-rvc-lui branch December 5, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants