[Flang] Exit gracefully with a useful message when we fail to lookup a target

[mnadeem]

Summary

Without this patch were asserting with a generic message Failed to create Target, but we already have a detailed error message stored in the variable error after calling lookupTarget() but this error was not getting used/printed.

With this patch we will emit a message with more details instead of a stack dump with a generic message.

[mnadeem]

flang/lib/Frontend/FrontendActions.cpp

599

I tried

unsigned diagID = ci.getDiagnostics().getCustomDiagID(clang::
           DiagnosticsEngine::Fatal, "unable to create target: '%0'");

ci.getDiagnostics().Report(diagID) << error;

it prints the error but does not terminate flang-new invocation for some reason.

[awarzynski]

flang/lib/Frontend/FrontendActions.cpp

598–600

You will need to update the function signature accordingly. Also, please add a test.

[awarzynski]

We already have an error message, so print it as well.

I'm not sure I follow - can you elaborate? This patch is switching from an assertion to a diagnostic. The error message inside the assert will be printed regardless of whether a diagnostic is used or not.

[mnadeem]

In D146333#4204007, @awarzynski wrote:

We already have an error message, so print it as well.

I'm not sure I follow - can you elaborate? This patch is switching from an assertion to a diagnostic. The error message inside the assert will be printed regardless of whether a diagnostic is used or not.

Before we were failing the assert with just a fixed message: "Failed to create Target".
With this patch we will also print the error description stored in the string error and then abort.

[mnadeem]

• Use diagnostics (reused existing diag)
• Add a test

[awarzynski]

In D146333#4204216, @mnadeem wrote:

In D146333#4204007, @awarzynski wrote:

We already have an error message, so print it as well.

I'm not sure I follow - can you elaborate? This patch is switching from an assertion to a diagnostic. The error message inside the assert will be printed regardless of whether a diagnostic is used or not.

Before we were failing the assert with just a fixed message: "Failed to create Target".
With this patch we will also print the error description stored in the string error and then abort.

Thanks for clarifying!

[awarzynski]

LGTM, thanks for improving this!

flang/test/Driver/target-machine-error.f90

1 ↗

(On Diff #506319)

[nit] IMHO, it would be easier to understand the intended logic in this test when using some random string (e.g. some-invalid-triple) rather than deliberately misspelling a valid triple (e.g. aarch67-linux-gnu) .

[tschuett]

flang/lib/Frontend/FrontendActions.cpp

615

Do you need to add a check here as well?

[mnadeem]

flang/lib/Frontend/FrontendActions.cpp

615

I briefly looked at the target code and it seems to me that this will only fail if the target does not support codgen or has not registered the target machine constructor. I think this is always expected to succeed.

flang/lib/Frontend/FrontendActions.cpp

void CodeGenAction::setUpTargetMachine() {
 
   const TargetOptions &targetOpts = ci.getInvocation().getTargetOpts();
   const std::string &theTriple = targetOpts.triple;
 
   // Create `Target`
   std::string error;
   const llvm::Target *theTarget =
       llvm::TargetRegistry::lookupTarget(theTriple, error);
-  assert(theTarget && "Failed to create Target");
+  if (!theTarget)
+    llvm::report_fatal_error(llvm::Twine("Failed to create Target: ") + error,
+                             false);
 
   // Create `TargetMachine`
   const auto &CGOpts = ci.getInvocation().getCodeGenOpts();
   std::optional<llvm::CodeGenOpt::Level> OptLevelOrNone =
       llvm::CodeGenOpt::getLevel(CGOpts.OptimizationLevel);
   assert(OptLevelOrNone && "Invalid optimization level!");
   llvm::CodeGenOpt::Level OptLevel = *OptLevelOrNone;
   std::string featuresStr = llvm::join(targetOpts.featuresAsWritten.begin(),
                                        targetOpts.featuresAsWritten.end(), ",");
   tm.reset(theTarget->createTargetMachine(
       theTriple, /*CPU=*/targetOpts.cpu,
       /*Features=*/featuresStr, llvm::TargetOptions(),
       /*Reloc::Model=*/CGOpts.getRelocationModel(),
       /*CodeModel::Model=*/std::nullopt, OptLevel));
   assert(tm && "Failed to create TargetMachine");
 }
 
 static std::unique_ptr<llvm::raw_pwrite_stream>
 getOutputStream(CompilerInstance &ci, llvm::StringRef inFile,
                 BackendActionTy action) {
   switch (action) {
   case BackendActionTy::Backend_EmitAssembly:
     return ci.createDefaultOutputFile(