[clangd] Add support for auxiliary triple specification

[oToToT]

Summary

I was working on CUDA support for clangd, and I've found that clangd is missing support for auxiliary triple specification. Thus, if we try making clangd work with CUDA or other languages that requires auxiliary triple, it will produce some wrong results.

After some investigation of what clang actually did, I borrowed some code from clang/lib/Frontend/CompilerInstance.cpp to make clangd work as well.

I don't know whether this fix is reasonable or not, but it just fix something looks like a problem in clangd.

[njames93]

Please upload diffs with full context. This can be done by passing -U999999 to diff or by using arcanist to upload your patches.

[oToToT]

re-upload diff with full context, sorry for that.

[oToToT]

clang/lib/Frontend/PrecompiledPreamble.cpp

311

Though this file is in clang, clangd use this function to prepare preamble in Preamble.cpp, and I think it is OK to add support for auxiliary triple here.

[sammccall]

Thanks for working this out!

Clearly there are some layering concerns here - clangd is "just" a clang tool that parses some code, and this is some pretty hairy clang internal logic we're copying here.
Practical concerns:

• it it could very easily skew from clang (e.g. will Sycl + CUDA + OpenMP always be the right list?!)
• there are other tools (callers of setTarget) that also need this logic
• this isn't even all the target logic we're missing, there's also fpenv stuff in CompilerInstance::ExecuteAction

Moreover this is entirely propagating settings from CompilerInvocation (spec) to CompilerInstance (state), which naturally belongs in ComplilerInstance.

I'd suggest this plan:

1. extract the target-related logic in CompilerInstance::ExecuteAction to a method createTarget(). This is NFC
2. update clangd + PrecompiledPreamble to call createTarget() instead of setTarget() +getTarget()->adjust() etc. This fixes this bug and others in clangd.
3. (optional, for you or me or others) update other callers of setTarget() in a similar way, which simplifies them and fixes bugs there too.

WDYT?

[oToToT]

Thanks sammccall for reviewing this and for the great idea of the whole plan.

I will work on the plan (maybe without the third one) to fix the problem described here.

Sorry for a poor submission here again.

[sammccall]

No apology necessary!

I think it's worth working out where best to put the fix, but the fix itself is the hard part from my perspective (not knowing much about cuda and other dialects). So this is much appreciated.

Please let me know if you hit any problems with the suggested refactoring, I don't want this to get lost.

clang-tools-extra/clangd/Compiler.cpp

if (auto VFSWithRemapping = createVFSFromCompilerInvocation(
     VFS = VFSWithRemapping;
   Clang->createFileManager(VFS);
 
   Clang->setTarget(TargetInfo::CreateTargetInfo(
       Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
   if (!Clang->hasTarget())
     return nullptr;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+       Clang->getLangOpts().SYCLIsDevice) &&
+      !Clang->getFrontendOpts().AuxTriple.empty()) {
+    auto TO = std::make_shared<TargetOptions>();
+    TO->Triple = llvm::Triple::normalize(Clang->getFrontendOpts().AuxTriple);
+    if (Clang->getFrontendOpts().AuxTargetCPU)
+      TO->CPU = Clang->getFrontendOpts().AuxTargetCPU.getValue();
+    if (Clang->getFrontendOpts().AuxTargetFeatures)
+      TO->FeaturesAsWritten =
+          Clang->getFrontendOpts().AuxTargetFeatures.getValue();
+    TO->HostTriple = Clang->getTarget().getTriple().str();
+    Clang->setAuxTarget(
+        TargetInfo::CreateTargetInfo(Clang->getDiagnostics(), TO));
+  }
+
+  // Inform the target of the language options.
+  //
+  // FIXME: We shouldn't need to do this, the target should be immutable once
+  // created. This complexity should be lifted elsewhere.
+  Clang->getTarget().adjust(Clang->getLangOpts());
+
+  if (auto *Aux = Clang->getAuxTarget())
+    Clang->getTarget().setAuxTarget(Aux);
+
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
   // release it.
   Buffer.release();
   return Clang;
 }
 
 } // namespace clangd
 } // namespace clang

clang/lib/Frontend/PrecompiledPreamble.cpp

 } // namespace
 
 PreambleBounds clang::ComputePreambleBounds(const LangOptions &LangOpts,
                                             const llvm::MemoryBufferRef &Buffer,
                                             unsigned MaxLines) {
   return Lexer::ComputePreamble(Buffer.getBuffer(), LangOpts, MaxLines);
 }
 
 llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
     const CompilerInvocation &Invocation,
     const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds,
     DiagnosticsEngine &Diagnostics,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
     std::shared_ptr<PCHContainerOperations> PCHContainerOps, bool StoreInMemory,
     PreambleCallbacks &Callbacks) {
   assert(VFS && "VFS is null");
 
llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
   Clang->setDiagnostics(&Diagnostics);
 
   // Create the target instance.
   Clang->setTarget(TargetInfo::CreateTargetInfo(
       Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
   if (!Clang->hasTarget())
     return BuildPreambleError::CouldntCreateTargetInfo;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+       Clang->getLangOpts().SYCLIsDevice) &&
+      !Clang->getFrontendOpts().AuxTriple.empty()) {
+    auto TO = std::make_shared<TargetOptions>();
+    TO->Triple = llvm::Triple::normalize(Clang->getFrontendOpts().AuxTriple);
+    if (Clang->getFrontendOpts().AuxTargetCPU)
+      TO->CPU = Clang->getFrontendOpts().AuxTargetCPU.getValue();
+    if (Clang->getFrontendOpts().AuxTargetFeatures)
+      TO->FeaturesAsWritten =
+          Clang->getFrontendOpts().AuxTargetFeatures.getValue();
+    TO->HostTriple = Clang->getTarget().getTriple().str();
+    Clang->setAuxTarget(
+        TargetInfo::CreateTargetInfo(Clang->getDiagnostics(), TO));
+  }
+
   // Inform the target of the language options.
   //
   // FIXME: We shouldn't need to do this, the target should be immutable once
   // created. This complexity should be lifted elsewhere.
   Clang->getTarget().adjust(Clang->getLangOpts());
 
+  if (auto *Aux = Clang->getAuxTarget())
+    Clang->getTarget().setAuxTarget(Aux);
+
   if (Clang->getFrontendOpts().Inputs.size() != 1 ||
       Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
           InputKind::Source ||
       Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() ==
           Language::LLVM_IR) {
     return BuildPreambleError::BadInputs;
   }