<div dir="ltr">Hi Eric,<br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 26, 2015 at 4:40 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Akira,<br><br><div>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.</div><div><br></div></blockquote><div><br></div><div>I saw your recent commits that untangle all of this. Thank you for your work!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div></div><div>FWIW the clang patch can be done about like this I believe (at least this is the working code I've been using):</div><div><br></div><div><div>+ // Add target-cpu and target-features work if they differ from the defaults.</div><div>+ // FIXME: Triple: x86_64-* versus CPU x86-64.</div><div>+ std::string &CPU = getTarget().getTargetOpts().CPU;</div><div>+ if (CPU != "" && CPU != getTarget().getTriple().getArchName())</div><div>+ FuncAttrs.addAttribute("target-cpu", getTarget().getTargetOpts().CPU);</div><div>+</div><div>+ std::vector<std::string> &Features =</div><div>+ getTarget().getTargetOpts().FeaturesAsWritten;</div><div>+ if (!Features.empty()) {</div><div>+ std::stringstream S;</div><div>+ std::copy(Features.begin(), Features.end(),</div><div>+ std::ostream_iterator<std::string>(S, " "));</div><div>+ // The drop_back gets rid of the trailing space.</div><div>+ FuncAttrs.addAttribute("target-features",</div><div>+ StringRef(S.str()).drop_back(1));</div><div>+ }</div></div><div><br></div></blockquote><div><br></div><div>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.</div><div> <br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div></div><div>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.</div><div><br></div><div>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.</div><span><font color="#888888"><div><br></div></font></span></blockquote><div><br></div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span><font color="#888888"><div></div><div>-eric</div></font></span><div><div><br><div class="gmail_quote">On Mon Jan 26 2015 at 4:28:41 PM Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>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).<br></div><div><br></div><div><a href="http://thread.gmane.org/gmane.comp.compilers.llvm.devel/78855" target="_blank">http://thread.gmane.org/gmane.comp.compilers.llvm.devel/78855</a><br></div><div><a href="http://thread.gmane.org/gmane.comp.compilers.llvm.devel/80456" target="_blank">http://thread.gmane.org/gmane.comp.compilers.llvm.devel/80456</a><br></div><div><br></div><div>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).</div><div><br></div><div>The attached patches make the following changes:</div><div><br></div><div>- 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.<br></div><div><br></div><div>- 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.</div><div><br></div><div>- Fix X86's subtarget lookup to override the function attributes if the corresponding options were specified on the command line.</div><div><br></div><div>- FIx clang to embed "-target-cpu" and "-target-feature" attributes in the IR.</div><div><br></div><div>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.</div><div><br></div><div>This is my plan for the remaining tasks:</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div></div>
</blockquote></div>
</div></div></blockquote></div><br></div></div>