[PATCH] D88748: [llc] Initialize TargetOptions after Triple is available

[Jason Liu via Phabricator via llvm-commits]

jasonliu accepted this revision.
jasonliu added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/tools/llc/llc.cpp:429
+  auto InitializeOptions = [&](Triple &TheTriple) {
+    Options = codegen::InitTargetOptionsFromCodeGenFlags();
+    Options.DisableIntegratedAS = NoIntegratedAssembler;
----------------
MaskRay wrote:
> jasonliu wrote:
> > Thanks for the patch. 
> > An ideal solution would be actually pass the Triple as argument to the `codegen::InitTargetOptionsFromCodeGenFlags`. So that InitTargetOptionsFromCodeGenFlags could always initialize the correct default value depending on the target triple if an explicit setting is not present. 
> > And we would do something similar to this:
> > Options.DataSections = codegen::getExplicitDataSections.getValueOr(Triple.hasDefaultDataSections());
> > 
> > Doing it after `codegen::InitTargetOptionsFromCodeGenFlags` gets called looks like an aftermath, and there could be tools other than llc called `codegen::InitTargetOptionsFromCodeGenFlags` and they should do this dance, but they do not. 
> Mentioned in the description: "This patch defers initialization of TargetOptions so that a future patch can pass TargetOptions to InitTargetOptionsFromCodeGenFlags"
> 
> The `InitTargetOptionsFromCodeGenFlags` change will be yours:) 
> 
> ---
> 
> I'll change `Triple &TheTriple` to `const Triple &TheTriple`.
Sounds good. Let me give it a try in that direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88748