[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