[RISCV] Disable generation of asynchronous unwind tables for RISCV baremetal #81727
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang-driver Author: Garvit Gupta (quic-garvgupt) ChangesThe below culprit patch enabled the generation of asynchronous unwind tables (-funwind-tables=2) by default for RISCV for both linux and RISCVToolChain baremetal object. However, since there are 2 baremetal toolchain objects for RISCV, this created a discrepancy between their behavior. Moreover, enabling the generation of asynchronous unwind tables based on whether Culprit Patch - https://reviews.llvm.org/D145164 Full diff: https://github.com/llvm/llvm-project/pull/81727.diff 3 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 85beb945cbf6fc..624099d21ae124 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -86,6 +86,11 @@ RISCVToolChain::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
return ToolChain::UNW_None;
}
+ToolChain::UnwindTableLevel RISCVToolChain::getDefaultUnwindTableLevel(
+ const llvm::opt::ArgList &Args) const {
+ return UnwindTableLevel::None;
+}
+
void RISCVToolChain::addClangTargetOptions(
const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
diff --git a/clang/lib/Driver/ToolChains/RISCVToolchain.h b/clang/lib/Driver/ToolChains/RISCVToolchain.h
index cec817ef7190be..fa0aa265d842bb 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -28,6 +28,8 @@ class LLVM_LIBRARY_VISIBILITY RISCVToolChain : public Generic_ELF {
RuntimeLibType GetDefaultRuntimeLibType() const override;
UnwindLibType
GetUnwindLibType(const llvm::opt::ArgList &Args) const override;
+ UnwindTableLevel
+ getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override;
void
AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
diff --git a/clang/test/Driver/clang-translation.c b/clang/test/Driver/clang-translation.c
index a7343ea18b2135..64d55f1b88ea6d 100644
--- a/clang/test/Driver/clang-translation.c
+++ b/clang/test/Driver/clang-translation.c
@@ -421,3 +421,11 @@
// MIPSN32R6EL: "-target-cpu" "mips64r6"
// MIPSN32R6EL: "-target-abi" "n32"
// MIPSN32R6EL: "-mfloat-abi" "hard"
+
+// RUN: %clang --target=riscv32-unknown-elf --gcc-toolchain="" -### %s 2>&1 | FileCheck %s -check-prefix=NOUWTABLE
+// RUN: %clang --target=riscv32-unknown-elf --gcc-toolchain="" -fasynchronous-unwind-tables -### %s 2>&1 | FileCheck %s -check-prefix=UWTABLE
+// RUN: %clang --target=riscv64-unknown-elf --gcc-toolchain="" -### %s 2>&1 | FileCheck %s -check-prefix=NOUWTABLE
+// RUN: %clang --target=riscv64-unknown-elf --gcc-toolchain="" -fasynchronous-unwind-tables -### %s 2>&1 | FileCheck %s -check-prefix=UWTABLE
+//
+// UWTABLE: "-funwind-tables=2"
+// NOUWTABLE-NOT: "-funwind-tables=2"
\ No newline at end of file
|
|
Hi |
dtcxzyw
requested review from
asb,
MaskRay,
kito-cheng,
apazos and
jonathonpenix
dtcxzyw
changed the title
|
I wonder whether GCC considers using |
|
RISC-V GCC has enabled NOTE: The patch[1] is come from SUSE folks, so I assume it's necessary from the distro's point of view. Ref: |
|
CC @petrhosek too who I know hopes to merge the RISCVToolChain and BareMetal toolchains. Matching the GCC behaviour seems sensible to me, and I'm hopeful that the planned future refactoring will avoid this kind of discrepancy in behaviour based on |
|
Hi @asb, since Petr already gave a good-ahead to this patch in the meeting, let me know if we can merge this PR if there are no new changes need to be made |
quic-garvgupt
force-pushed
the
RISCV_UnwindTables
branch
2 times, most recently
from
42211bc
to
8cc7535
Compare
|
Hi @asb, I do not have write access, requesting to merge this PR on my behalf. Also, requesting to backport this PR onto the release branch. Thanks in advance! |
…remetal The below culprit patch enabled the generation of asynchronous unwind tables (-funwind-tables=2) by default for RISCV for both linux and RISCVToolChain baremetal object. However, since there are 2 baremetal toolchain objects for RISCV, this created a discrepancy between their behavior. Moreover, enabling the generation of asynchronous unwind tables based on whether `-gcc-toolchain` option is present or not doesn't seem to be the best criteria to decide on the same. Culprit Patch - https://reviews.llvm.org/D145164
quic-garvgupt
force-pushed
the
RISCV_UnwindTables
branch
from
8cc7535
to
0d1f9be
Compare
|
Hi @MaskRay, please review the updated changes and if they are fine, please merge this PR on my behalf. Thanks! |
|
ping! for merging this PR |
|
@quic-garvgupt Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
The below culprit patch enabled the generation of asynchronous unwind tables (-funwind-tables=2) by default for RISCV for both linux and RISCVToolChain baremetal object. However, since there are 2 baremetal toolchain objects for RISCV, this created a discrepancy between their behavior. Moreover, enabling the generation of asynchronous unwind tables based on whether
-gcc-toolchainoption is present or not doesn't seem to be the best criteria to decide on the same. This patch make the behavior consistent by disabling the unwind tables in RISCVToolChain Baremetal object.Culprit Patch - https://reviews.llvm.org/D145164