Page MenuHomePhabricator

[Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables
ClosedPublic

Authored by MaskRay on Wed, Nov 18, 9:22 PM.

Details

Summary

In GCC, aarch64-*-linux and aarch64-*-freebsd made the switch in 2018
(https://gcc.gnu.org/pipermail/gcc-patches/2018-March/495549.html).
In Clang, FreeBSD/Fuchsia/NetBSD/MinGW aarch64 default to -fasynchronous-unwind-tables.

This patch defaults Generic_GCC aarch64 (which affects Linux) to use -fasynchronous-unwind-tables.

Diff Detail

Event Timeline

MaskRay created this revision.Wed, Nov 18, 9:22 PM
MaskRay requested review of this revision.Wed, Nov 18, 9:22 PM
MaskRay updated this revision to Diff 306312.Wed, Nov 18, 10:02 PM

Drop an unneeded change

Personally I'm in favour of clang and gcc behaving the same way unless there is a good reason not to. I've shared the review internally to see if anyone has any concerns. May be worth informing the clang built linux community to see if they will need to make any alterations as a result.

nickdesaulniers accepted this revision.Thu, Nov 19, 1:31 PM

Thanks for the heads up, I don't see this being a problem for us, especially since GCC already behaves this way;

arch/arm64/Makefile:
55 # Avoid generating .eh_frame* sections.
56 KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
57 KBUILD_AFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables

This revision is now accepted and ready to land.Thu, Nov 19, 1:31 PM

Also, built and boot tested an aarch64 defconfig just to be safe.

tstellar added a subscriber: tstellar.
joerg added a comment.Thu, Nov 19, 1:58 PM

I have no problem with the change, but please adjust the description to take about -funwind-tables. I don't think we do async by default, at least not by design.

MaskRay added a comment.EditedThu, Nov 19, 2:01 PM

I have no problem with the change, but please adjust the description to take about -funwind-tables. I don't think we do async by default, at least not by design.

AFAICT we don't make distinction of -funwind-tables and -fasynchronous-unwind-tables. I think the thing llvm implements is -fasynchronous-unwind-tables. What is the difference of -funwind-tables?

If you care, you may want to change GCC aarch64-*-netbsd to default to -fasynchronous-unwind-tables. I think it does not do it currently..

joerg added a comment.Thu, Nov 19, 2:55 PM

The difference is whether we promise to be instruction precise or not. I'm not sure we do or want to promise that as default.

The difference is whether we promise to be instruction precise or not. I'm not sure we do or want to promise that as default.

gcc/common/config/{aarch64,rs6000}/*-common.c enables -fasynchronous-unwind-tables so I think the title is right about the eventual state.
I think the current state for x86-64 is that the produced .cfi instructions satisfy the asynchronous requirement.
Other targets (e.g. aarch64) mostly satisfy the requirement, but their epilogues may miss some instructions due to missing hooks required by CFIInstrInserter. This is just a QoI issue (and if I'll probably work on this on my spare time).
(GCC has its problems, too. It does not analyze inline asm and cannot produce 100% asynchronous unwind tables)

I think it is right that in this patch we say we that we default to -fasynchronous-unwind-tables.

@psmith Do they have concerns? :)

Radio silence so far; I think no news is good news applies in this case. I'm happy to say no objections.

Radio silence so far; I think no news is good news applies in this case. I'm happy to say no objections.

Thanks. I'll commit then.

This revision was landed with ongoing or failed builds.Tue, Nov 24, 9:51 AM
This revision was automatically updated to reflect the committed changes.