This is an archive of the discontinued LLVM Phabricator instance.

[llc] Initialize TargetOptions after Triple is available
ClosedPublic

Authored by MaskRay on Oct 2 2020, 10:32 AM.

Details

Summary

Some targets have different defaults. This patch defers initialization of TargetOptions so that a future patch can pass TargetOptions to InitTargetOptionsFromCodeGenFlags

Diff Detail

Event Timeline

MaskRay created this revision.Oct 2 2020, 10:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 2 2020, 10:32 AM
MaskRay requested review of this revision.Oct 2 2020, 10:32 AM
MaskRay edited the summary of this revision. (Show Details)Oct 2 2020, 10:35 AM
jasonliu added inline comments.Oct 2 2020, 11:05 AM
llvm/tools/llc/llc.cpp
429

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.

MaskRay marked an inline comment as done.Oct 2 2020, 11:20 AM
MaskRay added inline comments.
llvm/tools/llc/llc.cpp
429

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.

jasonliu accepted this revision.Oct 2 2020, 11:33 AM
jasonliu added inline comments.
llvm/tools/llc/llc.cpp
429

Sounds good. Let me give it a try in that direction.

This revision is now accepted and ready to land.Oct 2 2020, 11:33 AM
This revision was landed with ongoing or failed builds.Oct 2 2020, 11:47 AM
This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as done.