[LLVMdev] Embedding cpu and feature strings into IR and enabling switching subtarget on a per function basis

[Akira Hatanaka]

Hi Eric,

On Mon, Jan 26, 2015 at 4:40 PM, Eric Christopher <echristo at gmail.com>
wrote:

> Hi Akira,
>
> Your patches are doing some of the things that are currently in progress,
> see the recent patches I've been applying in an attempt to untangle all of
> this.
>
>
I saw your recent commits that untangle all of this. Thank you for your
work!


> FWIW the clang patch can be done about like this I believe (at least this
> is the working code I've been using):
>
> +    // Add target-cpu and target-features work if they differ from the
> defaults.
> +    // FIXME: Triple: x86_64-* versus CPU x86-64.
> +    std::string &CPU = getTarget().getTargetOpts().CPU;
> +    if (CPU != "" && CPU != getTarget().getTriple().getArchName())
> +      FuncAttrs.addAttribute("target-cpu",
> getTarget().getTargetOpts().CPU);
> +
> +    std::vector<std::string> &Features =
> +        getTarget().getTargetOpts().FeaturesAsWritten;
> +    if (!Features.empty()) {
> +      std::stringstream S;
> +      std::copy(Features.begin(), Features.end(),
> +                std::ostream_iterator<std::string>(S, " "));
> +      // The drop_back gets rid of the trailing space.
> +      FuncAttrs.addAttribute("target-features",
> +                             StringRef(S.str()).drop_back(1));
> +    }
>
>
I noticed that this is setting the attributes in
CodeGenModule::ConstructAttributeList instead of in
CodeGenFunction::StartFunction as I did in my patch. This is probably the
correct thing to do. I wonder whether these attributes should be added to
function declarations, but that's another discussion.

Another difference is that this is passing TargetOptions::FeaturesAsWritten
to the IR instead of TargetOptions::Features. Since TargetOptions::Features
is passed to createTargetMachine in EmitAssemblyHelper::CreateTargetMachine
(and eventually passed to subtarget constructor), I was thinking
TargetOptions::Features should be written to the IR so that it can be used
later to construct per-function subtarget objects.

The first step is to remove all uses of the module level subtarget aspect.
> I've got the outline for this already done for various targets. The next
> step is to remove the use of getSubtargetImpl() and getSubtarget<>() and
> replace them with the ones that take the Functions. Part of your patch does
> this and is a reasonable first start to get things started (though not
> acceptable to go in as yet). Migrating things to
> FunctionTargetTransformInfo where applicable (i.e. anything that uses the
> stashed TargetLowering instance) would be a good start for you.
>
> Until then we can't go forward with the rest of the work. Your assistance
> is welcome in untangling the various bits of the infrastructure. I can
> highlight additional areas if you'd like.
>
>
I was also thinking it would be nice if the module level subtarget aspect
could be removed, but I didn't realize you were already working on doing
that. I think that will be a huge step towards fully enabling compiling
different functions with different options.


> -eric
>
> On Mon Jan 26 2015 at 4:28:41 PM Akira Hatanaka <ahatanak at gmail.com>
> wrote:
>
>> I've been investigating what is needed to ensure command line options are
>> passed to the backend codegen passes during LTO and enable compiling
>> different functions in a module with different command line options (see
>> the links below for previous discussions).
>>
>> http://thread.gmane.org/gmane.comp.compilers.llvm.devel/78855
>> http://thread.gmane.org/gmane.comp.compilers.llvm.devel/80456
>>
>> The command line options I'm currently looking into are "-target-cpu" and
>> "-target-feature" and I would like to get feedback about the approach I've
>> taken (patches attached).
>>
>> The attached patches make the following changes:
>>
>> - In TargetMachine::getSubtarget(const Function*) and MachineFunction's
>> constructor, use per-function subtarget object instead of TargetMachine's
>> (module-level) subtarget object. This allows passes like selection dag to
>> switch the target on a per-function basis.
>>
>> - Define class TargetOptions::Option, which records whether an option has
>> appeared on the command line along with the option's value. Long term, this
>> might not be the best solution and I expect it will be modified or replaced
>> when the new command line infrastructure becomes available.
>>
>> - Fix X86's subtarget lookup to override the function attributes if the
>> corresponding options were specified on the command line.
>>
>> - FIx clang to embed "-target-cpu" and "-target-feature" attributes in
>> the IR.
>>
>> I've tested the changes I made and confirmed that target options such as
>> "-mavx2" don't get dropped during LTO and are honored by backend codegen
>> passes.
>>
>> This is my plan for the remaining tasks:
>>
>> 1. FIx other in-tree targets and other code-gen passes that are still
>> using TargetMachine's subtarget where the per-function subtarget should be
>> used.
>>
>> 2. Fix TargetTransformInfo to compute the various code-gen costs
>> accurately when subtarget is switched on a per-function basis. One way to
>> do this is to make the pointer or reference to the Function object
>> available to the various subclasses of TargetTransformInfo by defining the
>> necessary functions in FunctionTargetTransformInfo (similar to the changes
>> made in r218004). However, passes like Inliner that are not function passes
>> cannot access FunctionTargetTransformInfo, so it has to be done in a
>> different way.
>>
>> 3. Forbid inlining functions that have incompatible cpu and feature
>> attributes. It seems the simplest approach is to allow inlining only if the
>> cpu and feature attributes match exactly, but it's also possible to relax
>> this restriction.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150127/12e9755b/attachment.html>