[clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[wanders]

Summary

The -mno-red-zone (and the option to cancel it out: -mred-zone) were not
allowed to be combined with -fembed-bitcode. However this option has a
direct effect on the generated bitcode as it adds the 'noredzone'
attribute on all functions. This means it is propagated into the
embedded bitcode and it is possible to regenerate the same object from
the embedded bitcode. Therefore there is no reason to forbid it.

[steven_wu]

The main concern for adding this blacklist was because of long term maintainability since the option is going to be embedded into bitcode. Looking at this specific option, I have no reason against because it doesn't affect embedded compiler flags.

I added Tim to see if allowing disable redzone can affect ABI and if it can be supported.

[compnerd]

@steven_wu - yeah, -mred-zone, -mno-red-zone does impact the ABI, at least on x86_64. It changes the way that the arguments are spilled and the stack layout.

[steven_wu]

In D61627#1493674, @compnerd wrote:

@steven_wu - yeah, -mred-zone, -mno-red-zone does impact the ABI, at least on x86_64. It changes the way that the arguments are spilled and the stack layout.

Well, I am not concern about ABI in that sense and not x86_64. I am concerning about ABI re-targeting from armv7k to arm64_32. It might be just fine but I can't tell for sure.

[compnerd]

Well, Apple's ARM64 ABI also has red zones and they are similar to x64 in the sense that they can be used for spilling locals. I believe that the ILP32 variant retains the red zones as well, so, yes, this would impact the ABI unfortunately, and the platform guarantees red zones.

[wanders]

But running code that has been compiled with -mno-red-zone on a platform that guarantees that redzones are not stamped on should be fine as far as I can understand?

The red zone is that the platform guarantees that it (e.g signal handlers, interrupts) will not use some memory outside of current stack pointer, so that a program can use that stack memory without adjusting the stackpointer. When compiling with -mno-red-zone the program will not use that but instead always adjust the stack pointer. So compiling an application with -mno-red-zone on a platform which allows red-zone usage will just cause some extra stack pointer adjustments, but not real ABI problems? On the other hand, compiling with -mred-zone (default) on a system that does not allow red-zone will risk strange corruptions when program has put data on stack without adjusting stackpointer and a signal/interrupt arrives.

If my reasoning there happens to be correct I think that -mno-red-zone should be safe to lift of the blacklist, but -mred-zone should be kept on the black list (at least if there is/can be such a thing as a platform where -mno-red-zone is the default)

I think this is similar to -mno-implicit-float which _is_ allowed to combined with -fembed-bitcode. One could easily envision a platform where fpu registers are not preserved on signal/interrupts. And then -mimplicit-float would be broken. However using -mno-implicit-float on a platform where the they are preserved is fine (except for possible performance loss).

I'm also a bit unsure about exact reasons for putting things in the blacklist. I'd also like to remove -mcmodel= from the blacklist. So maybe I should start a separate thread on cfe-dev on this for a more general discussion.

[steven_wu]

Like I said, I am not worried that -mno-red-zone itself changes the ABI. As long as LLVM still respect the attribute the same way, it is fine. I want to consult Tim to make sure we can support re-targeting no red zone from armv7k to arm64_32.

The intention for the black list is for maintainability. Limiting the ability to change the backend behavior means less corner case to worry about when we re-compile armv7k to arm64_32. Why are you interested in expending this list? Do you have any specific use case in mind? The other option is factor the blacklist out to be a Darwin toolchain specific overwrite so you can further relax it on other platforms.

[wanders]

Why are you interested in expending this list?

I have a (kernel) that is compiled with -mno-red-zone and -mcmodel=large which I want to compile with -fembed-bitcode for a debugging tool that needs the bitcode.

But I can carry these patches locally for now.

[steven_wu]

In D61627#1497919, @wanders wrote:

Why are you interested in expending this list?

I have a (kernel) that is compiled with -mno-red-zone and -mcmodel=large which I want to compile with -fembed-bitcode for a debugging tool that needs the bitcode.

But I can carry these patches locally for now.

I think the best option for this situation might be just make this blacklist to be darwin toolchain specific.

[wanders]

I might revisit this later. But carrying this patch locally for now.

clang/lib/Driver/ToolChains/Clang.cpp

static const constexpr unsigned kBitcodeOptionBlacklist[] = {
         options::OPT_mno_relax_all,
         options::OPT_ftrap_function_EQ,
         options::OPT_ffixed_r9,
         options::OPT_mfix_cortex_a53_835769,
         options::OPT_mno_fix_cortex_a53_835769,
         options::OPT_ffixed_x18,
         options::OPT_mglobal_merge,
         options::OPT_mno_global_merge,
-        options::OPT_mred_zone,
-        options::OPT_mno_red_zone,
         options::OPT_Wa_COMMA,
         options::OPT_Xassembler,
         options::OPT_mllvm,
     };
     for (const auto &A : Args)
       if (llvm::find(kBitcodeOptionBlacklist, A->getOption().getID()) !=
           std::end(kBitcodeOptionBlacklist))
         D.Diag(diag::err_drv_unsupported_embed_bitcode) << A->getSpelling();