[PATCH] D48756: Add option for section ordering

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Added Davide who made the change to make these the default (in r309056 and r309075) for thoughts. Should the default be true to be consistent with prior behavior? I've noted a bad interaction that I think will occur with the -ffunction-sections/-fdata-sections options below.



================
Comment at: tools/gold/gold-plugin.cpp:841
 
-  // Enable function/data sections by default.
-  Conf.Options.FunctionSections = true;
-  Conf.Options.DataSections = true;
+  Conf.Options.FunctionSections = options::section_ordering;
+  Conf.Options.DataSections = options::section_ordering;
----------------
I think unconditionally writing these options will interact badly with -ffunction-sections and -fdata-sections. Back in r284140 I added support to clang to pass down the internal -function-sections and -fdata-sections options when those driver options are set. From what I can tell, those internal llvm options will be applied to Conf.Options in InitTargetOptionsFromCodeGenFlags(), which is just above here (can't see it in phab - please upload patches with context).

So I think by default you would be always disabling those despite the internal llvm options being set. Can you confirm? Looks like the test I added with that patch just looked for the driver passing the internal option, so wouldn't catch this change in behavior.

Likely the clang side would need to change to set this gold plugin option instead.



Repository:
  rL LLVM

https://reviews.llvm.org/D48756