[ORC] Add host CPU name and sub-target features to JITTargetMachineBuilder::detectHost()

[dcaballe]

Summary

We use JITTargetMachineBuilder::detectHost() in MLIR to create a target machine for the host CPU and noticed that the returned JITTargetMachineBuilder is not properly initialized with the host CPU name and sub-target features. This patch adds that information to the JITTargetMachineBuilder created in JITTargetMachineBuilder::detectHost().

Please, let me know if there is a simple way to add a test for this.

Thanks!
Diego

[lhames]

Looks good to me. Thanks Diego!

I think it's ok to commit without a test for now. If detectHost() starts getting more complicated we could add a unit test to verify its behavior (basically: the resulting JTMB's fields match the values from getHostCPUName/getHostCPUFeatures/whatever-else-we-add). I don't think we need that yet though.

[dcaballe]

Thanks for the quick response, Lang! I'll proceed with the commit in a while, in case somebody else has any comments.

[plotfi]

I believe this change may be build-breaking:

http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/15952

[plotfi]

I've reproed this locally. Reverting.

[plotfi]

Reverted in r367954. Please try landing again @dcaballe

Please make sure to verify it builds on Linux.

[dcaballe]

Thanks, @plotfi. Sorry about that. It passed with gcc in my local machine. I'll check with Clang.

[plotfi]

In D65760#1616002, @dcaballe wrote:

Thanks, @plotfi. Sorry about that. It passed with gcc in my local machine. I'll check with Clang.

Ah yeah no worries. I've had it happen where the released clang worked fine locally but the bot Top-Of-Tree clang exposed a bug in my code before :-P.

llvm/trunk/lib/ExecutionEngine/Orc/JITTargetMachineBuilder.cpp

JITTargetMachineBuilder::JITTargetMachineBuilder(Triple TT)
     : TT(std::move(TT)) {
   Options.EmulatedTLS = true;
   Options.ExplicitEmulatedTLS = true;
 }
 
 Expected<JITTargetMachineBuilder> JITTargetMachineBuilder::detectHost() {
   // FIXME: getProcessTriple is bogus. It returns the host LLVM was compiled on,
   //        rather than a valid triple for the current process.
-  return JITTargetMachineBuilder(Triple(sys::getProcessTriple()));
+  JITTargetMachineBuilder TMBuilder(Triple(sys::getProcessTriple()));
+
+  // Retrieve host CPU name and sub-target features and add them to builder.
+  // Relocation model, code model and codegen opt level are kept to default
+  // values.
+  llvm::SubtargetFeatures SubtargetFeatures;
+  llvm::StringMap<bool> FeatureMap;
+  llvm::sys::getHostCPUFeatures(FeatureMap);
+  for (auto &Feature : FeatureMap)
+    SubtargetFeatures.AddFeature(Feature.first(), Feature.second);
+
+  TMBuilder.setCPU(llvm::sys::getHostCPUName());
+  TMBuilder.addFeatures(SubtargetFeatures.getFeatures());
+
+  return TMBuilder;
 }
 
 Expected<std::unique_ptr<TargetMachine>>
 JITTargetMachineBuilder::createTargetMachine() {
 
   std::string ErrMsg;
   auto *TheTarget = TargetRegistry::lookupTarget(TT.getTriple(), ErrMsg);
   if (!TheTarget)