Resurrect Bug18710 (Only generate .ARM.exidx and ARM.extab when needed with EHABI)

Hello Everyone,

This is my first attempt to getting used with the submission process. Trying to get the “good practice” with the coding standard, tools, mailing lists… and already a few questions:

  • Is it possible to “link” 2 related entries in Phabricator ? one for LLVM and one for CFE ? what’s the best way of posting 2 related or dependent patches ?

  • I’d like to update and follow on the discussion on bugzilla tracker that Renato created already but I can’t edit (account requested to without luck). Could someone help ?

So now the proposal:

The .exidx and .extab unwind sections are needed, when exceptions are enabled, for unwinding if the function can throw and to hold the EXIDX_CANTUNWIND value if the function does not throw.

The problem was already discussed in various threads and defects, so I won’t repeat the details but basically that confusion is that if the compiler (or the user) determined that the function cannot throw, we have the NOUNWIND attribute which is not orthogonal with either exception support. Despite this we should still emit the unwind entry for the EXIDX_CANTUNWIND value, which is always done.

Another issue is that canThrow (!NoUnwind) is set by default. Setting NoUnwind is an optimisation which is not handled by the ARM Streamer. A first approach was to have the backend return needsUnwindTableEntry() false if noexception support, but unwind tables can be used without exception handler (e.g) for debug infos or C unwind with attribute cleanup, and this happened to be quite intrusive…

But simpler, when the exceptions are not enabled, we should be able to remove them. Just passing this this bit of information through a new disable-arm-cantunwind llvm option makes the uwtable and nounwind table orthogonal with exception support.

This option is set in clang when no exception support allowing to have the -f[no-]exceptions and -f[no-]unwind-tables match GCC for C and C++.

Thanks a lot for your feedbacks, questions and hints for getting on board !



Christian

Often you can just link it in the description. I think Phab will even auto-connect them if you say somethig like “Depends on D12345”. If you look at some of chandlerc’s pass manager patches on Phab I know he was using Phab’s dependency feature for some of them.

Tanya, Anton? Could one of you help with the bugzilla account?

– Sean Silva

Often you can just link it in the description. I think Phab will even auto-connect them if you say somethig like “Depends on D12345”. If you look at some of chandlerc’s pass manager patches on Phab I know he was using Phab’s dependency feature for some of them.

Tanya, Anton? Could one of you help with the bugzilla account?

Has an account. Needs to reset password.

-Tanya

The description isn’t enough, you actually need to declare the dependency using "Edit Related Revisions…” in the column on the right.

But there an alternative for cross-project changes: using a single revision and a single commit! See: http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo

Also please remember to add llvm-commits and/or cfe-commits as subscribers when creating the revision so that the patch is sent to the mailing lists.

Best,

Often you can just link it in the description. I think Phab will even auto-connect them if you say somethig like “Depends on D12345”. If you look at some of chandlerc’s pass manager patches on Phab I know he was using Phab’s dependency feature for some of them.

The description isn’t enough, you actually need to declare the dependency using "Edit Related Revisions…” in the column on the right.

Putting ‘Depends on D1234’ in the summary during ‘arc diff’ has been working for me.

Hello,

After a few comments and rethought, it seems that the support needed to disable the cantunwind emission when the exception tables are not needed is best handled with a change of the uwtable attribute semantic rather than adding a new hidden flag.

So, for the ARM EHABI I propose the uwtable/nounwind semantic to be

nil (default) : : do not need an entry <- this is the change
nounwind : Function does not throw
uwtable : Need an entry because an exception might pass by
uwtable + nounwind : Need an entry even if no exception pass by

so "nil" really means "no table" and is orthogonal with uwtable.

then uwtable is only set when the exceptions are enabled (regardless of the language).
and nounwind is only set when the function cannot throw (regardless of the language).

This makes -f[no]-unwind-tables, -f[no]-exceptions effective

A (logical) consequence is that the unwind directives (.fnstart,.fnend) are only emitted if uwtable is set, so the few llvm tests that expected the uwtable attribute to be the default need to be adapted.

I've updated

https://reviews.llvm.org/D31139
https://reviews.llvm.org/D31140

(For some reasons, no notification was sent to the Subscribers (at least for llvm-commits). Maybe I'm still doing something wrong with Phabricator ?)

Thanks for your feedback,

Cheers

Christian