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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 2 11:20:05 PDT 2020


MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: llvm/tools/llc/llc.cpp:429
+  auto InitializeOptions = [&](Triple &TheTriple) {
+    Options = codegen::InitTargetOptionsFromCodeGenFlags();
+    Options.DisableIntegratedAS = NoIntegratedAssembler;
----------------
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`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88748



More information about the llvm-commits mailing list