[PATCH] D88737: [AIX] Turn -fdata-sections on by default in Clang

Fri Oct 2 09:35:13 PDT 2020

jasonliu added inline comments.

================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:47
+CODEGENOPT(DataSections      , 1, 0) ///< Set by default, or when -f[no-]data-sections.
+CODEGENOPT(HasExplicitDataSections, 1, 0) ///< Set when -f[no-]data-sections is set.
 CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-names.
----------------
jasonliu wrote:
> MaskRay wrote:
> > From the current code. I don't see HasExplicitDataSections is necessary. Please remove the lld changes.
> The reason I need `HasExplicitDataSections` goes back to D88493(which I haven't land yet, because I actually need to land these two patches together to avoid build break). In D88493, I'm trying to get the llc's default for -data-sections to be correct for AIX and introduced `HasExplicitDataSections`. If `HasExplicitDataSections` is not set, then llc would think there is -data-sections set, so it would just take the target triple's default, which makes `DataSections` setting to be useless. 
> It seems there is no good way to set a certain TargetOption's default to be dependant on the current target triple. As I already mentioned in my own comment in this patch, one of the way to achieve that could be make DataSections an enum option in TargetOptions, so that it could have a `Default` enum which could mean true or false depending on the triple setting. But it could mean a bigger refactoring to this option. 
Correction:
If HasExplicitDataSections is not set, then llc would think there is **no** -data-sections set, so it would just take the target triple's default...

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88737/new/

https://reviews.llvm.org/D88737