[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

[Quinn Pham via Phabricator via cfe-commits]

quinnp added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:579
+  else if (Args.hasArg(options::OPT_fno_data_sections))
+    CmdArgs.push_back("-plugin-opt=-data-sections=0");
 
----------------
MaskRay wrote:
> Is -plugin-opt=-data-sections=0 a problem for `!UseSeparateSections` targets?
I don't think `-plugin-opt=-data-sections=0` is a problem for `!UseSeparateSections` targets because we only add `"-plugin-opt=-data-sections=0"` if the user explicitly specified `-fno-data-sections`.  
- If `UseSeparateSections` is `true`, we will enter the  `if` block unless `-fno-data-sections` is explicitly set.
- If `UseSeparateSections` is `false`, we will enter the `if` block only if  `-fdata-sections` is explicitly set.

Then, if we did not enter the `if` block, we will only enter the `else if` block when `-fno-data-sections` is explicitly set.


================
Comment at: llvm/test/LTO/PowerPC/data-sections-linux.ll:20
+
+; CHECK-NO-DATA-SECTIONS-NOT: .var
+; CHECK-NO-DATA-SECTIONS:     0000000000000000 g O .bss {{.*}} var
----------------
MaskRay wrote:
> What does this `...-NOT: .var` do?
The line `; CHECK-NO-DATA-SECTIONS-NOT: .var` is to ensure that `data-sections` is correctly turned off. On Linux, when `data-sections` is on each variable `X` will have its own section named `.bss.X`. Here, I am trying to make sure that the the `.X` part is not present since it may have been consumed by the `{{.*}}` after the check for `.bss` on the next line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401