Skip to content
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

Revert "[Driver] Default riscv*- triples to -fdebug-default-version=4" #84119

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Mar 6, 2024

This reverts commit bbc0f99
(https://reviews.llvm.org/D157663).

With this change, -g for the next major release 19.1 will generate
R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128 relocations, which require
lld>=18 or binutils>=2.41.
binutils 2.41 is relatively new, but GCC has been producing
R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128 for some time now.

Created using spr 1.3.5-bogner
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 6, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 6, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Fangrui Song (MaskRay)

Changes

This reverts commit bbc0f99
(https://reviews.llvm.org/D157663).

With this change, the next major release 19.1 will generate
R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128 relocations, which require
lld>=18 or binutils>=2.41.
binutils 2.41 is relatively new, but GCC has been producing
R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128 for some time now.


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

3 Files Affected:

  • (modified) clang/include/clang/Driver/ToolChain.h (+1-1)
  • (modified) clang/lib/Driver/ToolChain.cpp (-6)
  • (modified) clang/test/Driver/clang-g-opts.c (-5)
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index fbe2e8fe8e88d8..a4f9cad98aa8b1 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -580,7 +580,7 @@ class ToolChain {
 
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
-  virtual unsigned GetDefaultDwarfVersion() const;
+  virtual unsigned GetDefaultDwarfVersion() const { return 5; }
 
   // Some toolchains may have different restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when host
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 08b1fd01b3c0ac..03450fc0f57b93 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -453,12 +453,6 @@ ToolChain::getDefaultUnwindTableLevel(const ArgList &Args) const {
   return UnwindTableLevel::None;
 }
 
-unsigned ToolChain::GetDefaultDwarfVersion() const {
-  // TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
-  // support becomes more widely available.
-  return getTriple().isRISCV() ? 4 : 5;
-}
-
 Tool *ToolChain::getClang() const {
   if (!Clang)
     Clang.reset(new tools::Clang(*this, useIntegratedBackend()));
diff --git a/clang/test/Driver/clang-g-opts.c b/clang/test/Driver/clang-g-opts.c
index b73602a155b009..fdbe0b96420c51 100644
--- a/clang/test/Driver/clang-g-opts.c
+++ b/clang/test/Driver/clang-g-opts.c
@@ -42,8 +42,3 @@
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
-
-/// TODO: Special case before R_RISCV_SET_ULEB128 linker support becomes more widely available.
-// RUN: %clang -### -S %s -g --target=riscv64-linux-gnu 2>&1 | FileCheck --check-prefix=VERSION4 %s
-// RUN: %clang -### -S %s -g --target=riscv64-unknown-elf 2>&1 | FileCheck --check-prefix=VERSION4 %s
-// VERSION4: "-dwarf-version=4"

@MaskRay
Copy link
Member Author

MaskRay commented Mar 13, 2024

Ping:)

Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

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

LGTM

@MaskRay MaskRay merged commit 63d53ee into main Mar 14, 2024
7 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/revert-driver-default-riscv-triples-to-fdebug-default-version4 branch March 14, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants