[Flang] Replace notifyMatchFailure with TODO hard failures

[kiranchandramohan]

Summary

For unimplemented patterns we revert to using TODO hard failures instead of
notifyMatchFailure.

For fir.select_type revert to using mlir::emiterror.
For the fir.embox TODO on a type with len params we cannot add a test since the type cannot be converted to llvm.

Adding negative tests using not and checking for the error message.
TODO exits with an error in a build without assertion but aborts in a
build with assertions. Abort requires using not with the --crash
option. The two different usages of not is handled by using a custom
command %not_todo_cmd which is converted to not or not --crash
depending on the presence or absence of assertions. Using llvm-config
to check the presence of assertions.

[awarzynski]

This really improves the documentation, thanks! I've noticed a few Ops without tests - perhaps it's not possible to test these? Worth adding a note in the summary.

Also, could you explain the rationale behind the change in the commit message? We discussed this on Slack, but IMO it would make sense to briefly sum up here too. For our future selves.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

786–787

How about a test for this one?

800–801

A test file?

1579

Test?

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

786–787

flang/test/Fir/Todo/dispatch.fir

800–801

flang/test/Fir/Todo/dispatch_table.fir

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1579

This is one of the case where the type conversion will failed first before reaching this point.

[awarzynski]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

786–787

My bad, missed it, thanks!

800–801

My bad, missed it, thanks!

1579

Ta! Could this be documented in the summary?

[mehdi_amini]

flang/test/Fir/Todo/end.fir

2

Can you add 2>&1 | FileCheck %s and check that you actually capture the expected reason to fail?

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1579

will do.

flang/test/Fir/Todo/end.fir

2

I am assuming you mean to prefix with not, redirect error to stdout and check with FileCheck after removing XFAIL, like given below.

As I mentioned in the other thread, this does not work in a debug build. In a debug build, fir::emitFatalError is called which calls mlir::emitError and then aborts with llvm_report_fatal_error. I think the abort makes it difficult to test.
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/include/flang/Lower/Todo.h#L65
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/include/flang/Optimizer/Support/FatalError.h#L23

// RUN: not fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
  
// Test `fir.end` conversion to llvm.
// Not implemented yet.

// CHECK: not yet implemented fir.end codegen

"fir.end"() : () -> ()

[mehdi_amini]

flang/test/Fir/Todo/end.fir

2

I am assuming you mean to prefix with not, redirect error to stdout and check with FileCheck after removing XFAIL, like given below.

The not prefix vs XFAIL is orthogonal to checking the message, my comment here is only about checking the cause of the failure.

In a debug build, fir::emitFatalError is called which calls mlir::emitError and then aborts with llvm_report_fatal_error. I think the abort makes it difficult to test.

I'm puzzled: wouldn't the message show up anyway?
Why wouldn't the // CHECK: not yet implemented fir.end codegen work in both mode?

2

The not prefix vs XFAIL is orthogonal to checking the message, my comment here is only about checking the cause of the failure.

Actually, no it isn't orthogonal. But the question remains: I'm not sure why the message isn't emitted in both cases.

[kiranchandramohan]

flang/test/Fir/Todo/end.fir

2

The message is emitted in both cases. The failure in debug build happens because not fails on abort.

I see that there is a --crash option in not to handle this situation. I will give this a shot. This might require separate lines/tests for debug vs non-debug builds.

[mehdi_amini]

flang/test/Fir/Todo/end.fir

2

Oh OK that's what I missed: I didn't know that not would not accept the crash situation by default!
Maybe TODO can emit a "crashing error code" in non-debug build?

[mehdi_amini]

flang/test/Fir/Todo/end.fir

2

Maybe TODO can emit a "crashing error code" in non-debug build?

Actually this may just not be possible, I'm not sure how we differentiate here in not.

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1184–1186

This one is actually not a TODO. This high level op is not likely to have a codegen to LLVM IR. It will be converted before to lower level ops.

[awarzynski]

flang/test/Fir/Todo/end.fir

2

If the behavior is different for Debug and Release builds, then we could define %fir_opt_todo` in LIT that expands to different things depending on the build type.

[kiranchandramohan]

For fir.select_type revert to using mlir::emiterror.

Adding negative tests using not and checking for the error message.
TODO exits with an error in a build without assertion but aborts in a
build with assertions. Abort requires using not with the --crash
option. The two different usages of not is handled by using a custom
command %not_cmd which is converted to not or not --crash
depending on the presence or absence of assertions. Using llvm-config
to check the presence of assertions.

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1184–1186

Done. Thanks.

1579

Done.

flang/test/Fir/Todo/end.fir

2

I am going with @awarzynski's suggestion. Instead of %fir_opt_todo I am using %not_cmd.

[mehdi_amini]

flang/test/Fir/Todo/end.fir

2

Nice trick!

[awarzynski]

I've left a few more nits - feel free to ignore. Thanks for implementing this! I think that you've also addressed all of Mehdi's comments, but please give it another day before merging (in case we missed sth).

flang/lib/Optimizer/CodeGen/CodeGen.cpp

545

FYI, I tried removing return failure(); and tested with

• -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=On, and
• -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=Off.

No compiler errors/warnings in both cases. But it's probably safer to leave return failure(); here.

flang/test/lit.cfg.py

38

This will only check for "asserts", right? Build type affects whether "asserts" are enabled, but that's just an implementation detail here. At this stage we just want to know whether the 'asserts' are "On", right?

78–87

[nit] It would be helpful to have a comment explaining "why" we may need not with --crash (and, ultimately, %not_cmd).

80

[nit] Wouldn't %not_or_crash be more descriptive than %not_cmd? Both not and not --crash are "commands", but:

//   not cmd
//     Will return true if cmd doesn't crash and returns false.
//   not --crash cmd
//     Will return true if cmd crashes (e.g. for testing crash reporting).

(from not.cpp). But perhaps that's more confusing?

[kiranchandramohan]

Rename not_cmd as not_todo_cmd. Add comments.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

545

OK. I will retain, incase some compiler warns.

flang/test/lit.cfg.py

38

Thanks. Yes, this is a copy paste error, there is another flag (build-type) that we can query but as u point out, we do not use that here.

80

Yes, I agree that not_cmd is not very descriptive. Since this is specific to the todo messages, I am replacing it with not_todo_cmd. This gives a hint that a developer should use not_todo_cmd when todo messages are involved. This along with the description should hopefully be enough.

[kiranchandramohan]

Add the llvm install directory for finding tools in out of tree builds

flang/lib/Optimizer/CodeGen/CodeGen.cpp

 /// boxproc.
 /// TODO: Part of supporting Fortran 2003 procedure pointers.
 struct BoxProcHostOpConversion : public FIROpConversion<fir::BoxProcHostOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::BoxProcHostOp boxprochost, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        boxprochost, "fir.boxproc_host codegen is not implemented yet");
+    TODO(boxprochost.getLoc(), "fir.boxproc_host codegen");
+    return failure();
   }
 };
 
 /// Lower `fir.box_tdesc` to the sequence of operations to extract the type
 /// descriptor from the box.
 struct BoxTypeDescOpConversion : public FIROpConversion<fir::BoxTypeDescOp> {
   using FIROpConversion::FIROpConversion;
 

 /// Lower `fir.dispatch` operation. A virtual call to a method in a dispatch
 /// table.
 struct DispatchOpConversion : public FIROpConversion<fir::DispatchOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::DispatchOp dispatch, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        dispatch, "fir.dispatch codegen is not implemented yet");
+    TODO(dispatch.getLoc(), "fir.dispatch codegen");
+    return failure();
   }
 };
 
 /// Lower `fir.dispatch_table` operation. The dispatch table for a Fortran
 /// derived type.
 struct DispatchTableOpConversion
     : public FIROpConversion<fir::DispatchTableOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::DispatchTableOp dispTab, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        dispTab, "fir.dispatch_table codegen is not implemented yet");
+    TODO(dispTab.getLoc(), "fir.dispatch_table codegen");
+    return failure();
   }
 };
 
 /// Lower `fir.dt_entry` operation. An entry in a dispatch table; binds a
 /// method-name to a function.
 struct DTEntryOpConversion : public FIROpConversion<fir::DTEntryOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::DTEntryOp dtEnt, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        dtEnt, "fir.dt_entry codegen is not implemented yet");
+    TODO(dtEnt.getLoc(), "fir.dt_entry codegen");
+    return failure();
   }
 };
 
 /// Lower `fir.global_len` operation.
 struct GlobalLenOpConversion : public FIROpConversion<fir::GlobalLenOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::GlobalLenOp globalLen, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        globalLen, "fir.global_len codegen is not implemented yet");
+    TODO(globalLen.getLoc(), "fir.global_len codegen");
+    return failure();
   }
 };
 
 /// Lower fir.len_param_index
 struct LenParamIndexOpConversion
     : public FIROpConversion<fir::LenParamIndexOp> {
   using FIROpConversion::FIROpConversion;
 
   // FIXME: this should be specialized by the runtime target
   mlir::LogicalResult
   matchAndRewrite(fir::LenParamIndexOp lenp, OpAdaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        lenp, "fir.len_param_index codegen is not implemented yet");
+    TODO(lenp.getLoc(), "fir.len_param_index codegen");
   }
 };
 
 /// Lower `fir.gentypedesc` to a global constant.
 struct GenTypeDescOpConversion : public FIROpConversion<fir::GenTypeDescOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::GenTypeDescOp gentypedesc, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        gentypedesc, "fir.fir.gentypedesc codegen is not implemented yet");
+    TODO(gentypedesc.getLoc(), "fir.gentypedesc codegen");
+    return failure();
   }
 };
 
 /// Convert `fir.end`
 struct FirEndOpConversion : public FIROpConversion<fir::FirEndOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::FirEndOp firEnd, OpAdaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        firEnd, "fir.end codegen is not implemented yet");
+    TODO(firEnd.getLoc(), "fir.end codegen");
+    return failure();
   }
 };
 
 /// Lower `fir.has_value` operation to `llvm.return` operation.
 struct HasValueOpConversion : public FIROpConversion<fir::HasValueOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
struct SelectCaseOpConversion : public FIROpConversion<fir::SelectCaseOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::SelectCaseOp caseOp, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     unsigned conds = caseOp.getNumConditions();
     llvm::ArrayRef<mlir::Attribute> cases = caseOp.getCases().getValue();
     // Type can be CHARACTER, INTEGER, or LOGICAL (C1145)
-    LLVM_ATTRIBUTE_UNUSED auto ty = caseOp.getSelector().getType();
-    if (ty.isa<fir::CharacterType>())
-      return rewriter.notifyMatchFailure(caseOp,
-                                         "conversion of fir.select_case with "
-                                         "character type not implemented yet");
+    auto ty = caseOp.getSelector().getType();
+    if (ty.isa<fir::CharacterType>()) {
+      TODO(caseOp.getLoc(), "fir.select_case codegen with character type");
+      return failure();
+    }
     mlir::Value selector = caseOp.getSelector(adaptor.getOperands());
     auto loc = caseOp.getLoc();
     for (unsigned t = 0; t != conds; ++t) {
       mlir::Block *dest = caseOp.getSuccessor(t);
       llvm::Optional<mlir::ValueRange> destOps =
           caseOp.getSuccessorOperands(adaptor.getOperands(), t);
       llvm::Optional<mlir::ValueRange> cmpOps =
           *caseOp.getCompareOperands(adaptor.getOperands(), t);

 
 /// Lower `fir.select_type` to LLVM IR dialect.
 struct SelectTypeOpConversion : public FIROpConversion<fir::SelectTypeOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::SelectTypeOp select, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        select, "fir.select_type codegen is not implemented yet");
+    mlir::emitError(select.getLoc(),
+                    "fir.select_type should have already been converted");
+    return failure();
   }
 };
 
 /// conversion of fir::SelectRankOp to an if-then-else ladder
 struct SelectRankOpConversion : public FIROpConversion<fir::SelectRankOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult

 };
 
 struct ZeroOpConversion : public FIROpConversion<fir::ZeroOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::ZeroOp zero, OpAdaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    auto ty = convertType(zero.getType());
+    mlir::Type ty = convertType(zero.getType());
     if (ty.isa<mlir::LLVM::LLVMPointerType>()) {
       rewriter.replaceOpWithNewOp<mlir::LLVM::NullOp>(zero, ty);
     } else if (ty.isa<mlir::IntegerType>()) {
       rewriter.replaceOpWithNewOp<mlir::LLVM::ConstantOp>(
           zero, ty, mlir::IntegerAttr::get(zero.getType(), 0));
     } else if (mlir::LLVM::isCompatibleFloatingPointType(ty)) {
       rewriter.replaceOpWithNewOp<mlir::LLVM::ConstantOp>(
           zero, ty, mlir::FloatAttr::get(zero.getType(), 0.0));
struct EmboxOpConversion : public EmboxCommonConversion<fir::EmboxOp> {
   matchAndRewrite(fir::EmboxOp embox, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     assert(!embox.getShape() && "There should be no dims on this embox op");
     auto [boxTy, dest, eleSize] =
         consDescriptorPrefix(embox, rewriter, /*rank=*/0,
                              /*lenParams=*/adaptor.getOperands().drop_front(1));
     dest = insertBaseAddress(rewriter, embox.getLoc(), dest,
                              adaptor.getOperands()[0]);
-    if (isDerivedTypeWithLenParams(boxTy))
-      return rewriter.notifyMatchFailure(
-          embox, "fir.embox codegen of derived with length parameters not "
-                 "implemented yet");
+    if (isDerivedTypeWithLenParams(boxTy)) {
+      TODO(embox.getLoc(),
+           "fir.embox codegen of derived with length parameters");
+      return failure();
+    }
     auto result = placeInMemoryIfNotGlobalInit(rewriter, embox.getLoc(), dest);
     rewriter.replaceOp(embox, result);
     return success();
   }
 };
 
 /// Lower `fir.emboxproc` operation. Creates a procedure box.
 /// TODO: Part of supporting Fortran 2003 procedure pointers.
 struct EmboxProcOpConversion : public FIROpConversion<fir::EmboxProcOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::EmboxProcOp emboxproc, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        emboxproc, "fir.emboxproc codegen is not implemented yet");
+    TODO(emboxproc.getLoc(), "fir.emboxproc codegen");
+    return failure();
   }
 };
 
-
 // Code shared between insert_value and extract_value Ops.
 struct ValueOpCommon {
   // Translate the arguments pertaining to any multidimensional array to
   // row-major order for LLVM-IR.
   static void toRowMajor(SmallVectorImpl<mlir::Attribute> &attrs,
                          mlir::Type ty) {
     assert(ty && "type is null");
     const auto end = attrs.size();

 /// components.
 /// TODO: Part of supporting Fortran 2003 procedure pointers.
 struct UnboxProcOpConversion : public FIROpConversion<fir::UnboxProcOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::UnboxProcOp unboxproc, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    return rewriter.notifyMatchFailure(
-        unboxproc, "fir.unboxproc codegen is not implemented yet");
+    TODO(unboxproc.getLoc(), "fir.unboxproc codegen");
+    return failure();
   }
 };
 
 /// Convert `fir.field_index`. The conversion depends on whether the size of
 /// the record is static or dynamic.
 struct FieldIndexOpConversion : public FIROpConversion<fir::FieldIndexOp> {
   using FIROpConversion::FIROpConversion;
 

flang/test/CMakeLists.txt

configure_lit_site_cfg(
   MAIN_CONFIG
   ${CMAKE_CURRENT_SOURCE_DIR}/NonGtestUnit/lit.cfg.py
 )
 
 set(FLANG_TEST_PARAMS
   flang_site_config=${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py)
 
 set(FLANG_TEST_DEPENDS
-  flang-new FileCheck count not module_files fir-opt tco
+  flang-new llvm-config FileCheck count not module_files fir-opt tco
 )
 
 if (FLANG_INCLUDE_TESTS)
   if (FLANG_GTEST_AVAIL)
     list(APPEND FLANG_TEST_DEPENDS FlangUnitTests)
   endif()
 endif()
 

flang/test/Fir/Todo/boxproc_host.fir

+// RUN: not fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test that `fir.boxproc_host` fails conversion to llvm.
+// At the moment this test fails since `fir.boxproc` type does not have a conversion.
+
+// CHECK: failed to legalize operation 'builtin.func'
+func @test(%bproc: !fir.boxproc<(i32) -> ()>) {
+  %tuple = fir.boxproc_host %bproc : (!fir.boxproc<(i32) -> ()>) -> (!fir.ref<tuple<i32,f64>>)
+  return
+}

flang/test/Fir/Todo/dispatch.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.dispatch` conversion to llvm.
+// Not implemented yet.
+
+func @dispatch(%arg0: !fir.box<!fir.type<derived3{f:f32}>>) {
+// CHECK: not yet implemented fir.dispatch codegen
+  %0 = fir.dispatch "method"(%arg0) : (!fir.box<!fir.type<derived3{f:f32}>>) -> i32
+  return
+}

flang/test/Fir/Todo/dispatch_table.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test fir.dispatch_table conversion to llvm.
+// Not implemented yet.
+
+// CHECK: not yet implemented fir.dispatch_table codegen
+fir.dispatch_table @dispatch_tbl {
+  fir.dt_entry "method", @method_impl
+}

flang/test/Fir/Todo/emboxproc.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.emboxproc` conversion to llvm.
+// Not implemented yet.
+
+func @emboxproc_test() {
+  %host_vars = fir.alloca tuple<i32,f64>
+// CHECK: not yet implemented fir.emboxproc codegen
+  %bproc = fir.emboxproc @method_impl, %host_vars : ((i32) -> (), !fir.ref<tuple<i32,f64>>) -> !fir.boxproc<(i32) -> ()>
+  return
+}

flang/test/Fir/Todo/end.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.end` conversion to llvm.
+// Not implemented yet.
+
+func @end_test() {
+// CHECK: not yet implemented fir.end codegen
+  "fir.end"() : () -> ()
+}

flang/test/Fir/Todo/gentypedesc.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.gentypedesc` conversion to llvm.
+// Not implemented yet.
+
+func @gentypedesc() {
+// CHECK: not yet implemented fir.gentypedesc codegen
+  %0 = fir.gentypedesc !fir.type<derived3>
+  return
+}

flang/test/Fir/Todo/global_len.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.global_len` conversion to llvm.
+// Not implemented yet.
+
+fir.global @global_derived : !fir.type<minez(f:i32)> {
+// CHECK: not yet implemented fir.global_len codegen
+  fir.global_len f, 1 : i32
+  %0 = fir.undefined !fir.type<minez>
+  fir.has_value %0 : !fir.type<minez>
+}

flang/test/Fir/Todo/len_param_index.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.len_param_index` conversion to llvm.
+// Not implemented yet.
+
+func @lenparamindex() {
+  // CHECK: not yet implemented fir.len_param_index codegen
+  %0 = fir.len_param_index l1, !fir.type<twolens(l1:i32, l2:i32){i:i32, f:f32, l:i64}>
+  return
+}
+

flang/test/Fir/Todo/select_case_with_character.fir

+// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.select_case` conversion to llvm with character type.
+// Not implemented yet.
+
+func @select_case_charachter(%arg0: !fir.char<2, 10>, %arg1: !fir.char<2, 10>, %arg2: !fir.char<2, 10>) {
+// CHECK: not yet implemented fir.select_case codegen with character type
+  fir.select_case %arg0 : !fir.char<2, 10> [#fir.point, %arg1, ^bb1,
+                                            #fir.point, %arg2, ^bb2,
+                                            unit, ^bb3]
+^bb1:
+  %c1_i32 = arith.constant 1 : i32
+  br ^bb3
+^bb2:
+  %c2_i32 = arith.constant 2 : i32
+  br ^bb3
+^bb3:
+  return
+}

flang/test/Fir/Todo/unboxproc.fir

+// RUN: not fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+
+// Test `fir.unboxproc` conversion to llvm.
+// Not implemented yet.
+// Currently fails since coversion for boxproc type is not implemented.
+
+// CHECK: failed to legalize operation 'builtin.func'
+func @boxing_match(%bproc: !fir.boxproc<(i32) -> ()>) {
+  %ubproc:2 = fir.unboxproc %bproc : (!fir.boxproc<(i32) -> ()>) -> ((i32) -> (), !fir.ref<tuple<i32,f64>>)
+  return
+}

flang/test/Fir/convert-to-llvm-invalid.fir

 // Test FIR to LLVM IR conversion invalid cases and diagnostics.
 
 // RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" --verify-diagnostics %s
 
 // Test `fir.zero` conversion failure with aggregate type.
 // Not implemented yet.
 
 func @zero_aggregate() {
   // expected-error@+1{{failed to legalize operation 'fir.zero_bits'}}
   %a = fir.zero_bits !fir.array<10xf32>
   return
 }
 
 // -----
 
-// Test that `fir.dispatch` fails to be legalized. Not implemented yet.
-
-func @dispatch(%arg0: !fir.box<!fir.type<derived3{f:f32}>>) {
-  // expected-error@+1{{failed to legalize operation 'fir.dispatch'}}
-  %0 = fir.dispatch "method"(%arg0) : (!fir.box<!fir.type<derived3{f:f32}>>) -> i32
-  return
-}
-
-// -----
-
-// Test that `fir.dispatch_table`/`fir.dt_entry` fails to be legalized.
-// Not implemented yet.
-
-// expected-error@+1{{failed to legalize operation 'fir.dispatch_table'}}
-fir.dispatch_table @dispatch_tbl {
-  fir.dt_entry "method", @method_impl
-}
-
-// -----
-
-// Test `fir.select_case` conversion failure with character type.
-// Not implemented yet.
-
-func @select_case_charachter(%arg0: !fir.char<2, 10>, %arg1: !fir.char<2, 10>, %arg2: !fir.char<2, 10>) {
-  // expected-error@+1{{failed to legalize operation 'fir.select_case'}}
-  fir.select_case %arg0 : !fir.char<2, 10> [#fir.point, %arg1, ^bb1,
-                                            #fir.point, %arg2, ^bb2,
-                                            unit, ^bb3]
-^bb1:
-  %c1_i32 = arith.constant 1 : i32
-  br ^bb3
-^bb2:
-  %c2_i32 = arith.constant 2 : i32
-  br ^bb3
-^bb3:
-  return
-}
-
-// -----
-
-// Test `fir.select_type` conversion failure. Not implemented yet.
-
-func @bar_select_type(%arg : !fir.box<!fir.type<derived3{f:f32}>>) -> i32 {
-  %0 = arith.constant 1 : i32
-  %2 = arith.constant 3 : i32
-
-  // expected-error@+1{{failed to legalize operation 'fir.select_type'}}
-  fir.select_type %arg : !fir.box<!fir.type<derived3{f:f32}>> [
-    #fir.instance<!fir.int<4>>,^bb1(%0:i32),
-    #fir.instance<!fir.int<8>>,^bb2(%2:i32),
-    unit,^bb5 ]
-^bb1(%a : i32) :
-  return %a : i32
-^bb2(%b : i32) :
-  return %b : i32
-^bb5 :
-  %zero = arith.constant 0 : i32
-  return %zero : i32
-}
-
-// -----
-
-// Test `fir.global_len` conversion failure. Not implemented yet.
-
-fir.global @global_derived : !fir.type<minez(f:i32)> {
-  // expected-error@+1{{failed to legalize operation 'fir.global_len'}}
-  fir.global_len f, 1 : i32
-  %0 = fir.undefined !fir.type<minez>
-  fir.has_value %0 : !fir.type<minez>
-}
-
-// -----
-
-// Test `fir.len_param_index` conversion failure. Not implemented yet.
-
-func @lenparamindex() {
-  // expected-error@+1{{failed to legalize operation 'fir.len_param_index'}}
-  %0 = fir.len_param_index l1, !fir.type<twolens(l1:i32, l2:i32){i:i32, f:f32, l:i64}>
-  return
-}
-
-// -----
-
-// Test `fir.gentypedesc` conversion failure. Not implemented yet.
-
-func @gentypedesc() {
-  // expected-error@+1{{failed to legalize operation 'fir.gentypedesc'}}
-  %0 = fir.gentypedesc !fir.type<derived3>
-  return
-}
-
-// -----
-
 // Verify that `fir.dt_entry` requires a parent op
 
 // expected-error@+1{{'fir.dt_entry' op expects parent op 'fir.dispatch_table'}}
 fir.dt_entry "method", @method_impl
 
 // -----
 
 // Test `fir.shape` conversion failure because the op has uses.
func @shape_shift_not_dead(%arg0: !fir.ref<!fir.array<?x?xf32>>, %i: index, %j: index) {
   // expected-error@+1{{failed to legalize operation 'fir.shape_shift'}}
   %0 = fir.shape_shift %c0, %c0, %c0, %c0 : (index, index, index, index) -> !fir.shapeshift<2>
   %1 = fir.array_coor %arg0(%0) %i, %j : (!fir.ref<!fir.array<?x?xf32>>, !fir.shapeshift<2>, index, index) -> !fir.ref<f32>
   return
 }
 
 // -----
 
-// Test that the type `fir.boxproc` fails to be legalized.
-// Not implemented yet.
-
-// expected-error@+1{{failed to legalize operation 'builtin.func'}}
-func private @foo0(%arg0: !fir.boxproc<() -> ()>)
-
-// -----
-
-// Test that `fir.emboxproc` fails to be legalized.
-// Not implemented yet.
+// Test `fir.select_type` conversion to llvm.
+// Should have been converted.
 
-func private @method_impl(i32)
-
-func @emboxproc_test() {
-  %host_vars = fir.alloca tuple<i32,f64>
-// expected-error@+1{{failed to legalize operation 'fir.emboxproc'}}
-  %bproc = fir.emboxproc @method_impl, %host_vars : ((i32) -> (), !fir.ref<tuple<i32,f64>>) -> !fir.boxproc<(i32) -> ()>
-  return
+func @bar_select_type(%arg : !fir.box<!fir.ref<f32>>) -> i32 {
+  %0 = arith.constant 1 : i32
+  %2 = arith.constant 3 : i32
+  // expected-error@+2{{fir.select_type should have already been converted}}
+  // expected-error@+1{{failed to legalize operation 'fir.select_type'}}
+  fir.select_type %arg : !fir.box<!fir.ref<f32>> [
+    #fir.instance<!fir.int<4>>,^bb1(%0:i32),
+    #fir.instance<!fir.int<8>>,^bb2(%2:i32),
+    unit,^bb5 ]
+^bb1(%a : i32) :
+  return %a : i32
+^bb2(%b : i32) :
+  return %b : i32
+^bb5 :
+  %zero = arith.constant 0 : i32
+  return %zero : i32
 }
-
-// Test that `fir.unboxproc` and `fir.boxproc_host` also fails to be legalized.
-// At the moment these cannot be tested since the `fir.boxproc` type does not have a conversion.
-
-// -----
-
-// Test `fir.end` conversion failure. Not implemented yet.
-
-// expected-error@+1{{failed to legalize operation 'fir.end'}}
-"fir.end"() : () -> ()

flang/test/lit.cfg.py

config.suffixes = ['.c', '.cpp', '.f', '.F', '.ff', '.FOR', '.for', '.f77', '.f90', '.F90',
                    '.CUF', '.f18', '.F18', '.fir', '.f03', '.F03', '.f08', '.F08']
 
 config.substitutions.append(('%PATH%', config.environment['PATH']))
 config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
 config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
 
 llvm_config.use_default_substitutions()
 
+# ask llvm-config about asserts
+llvm_config.feature_config(
+    [('--assertion-mode', {'ON': 'asserts'})])
+
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
 # directories.
 config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt']
 
 # If the flang examples are built, add examples to the config
 if config.flang_examples:
     config.available_features.add('examples')
if config.flang_llvm_tools_dir != "":
             llvm_config.with_environment('PATH', config.flang_llvm_tools_dir, append_path=True)
 
 # For each occurrence of a flang tool name, replace it with the full path to
 # the build directory holding that tool.
 tools = [
         ToolSubst('%flang', command=FindTool('flang-new'), unresolved='fatal'),
     ToolSubst('%flang_fc1', command=FindTool('flang-new'), extra_args=['-fc1'],
         unresolved='fatal')]
 
+# Flang has several unimplemented features. TODO messages are used to mark and fail if these
+# features are exercised. TODOs exit with an error in non-assert builds but in assert builds
+# it aborts. To catch aborts, the `--crash` option for the `not` command has to be used.
+if 'asserts' in config.available_features:
+    tools.append(ToolSubst('%not_todo_cmd', command=FindTool('not'), extra_args=['--crash'],
+        unresolved='fatal'))
+else:
+    tools.append(ToolSubst('%not_todo_cmd', command=FindTool('not'), unresolved='fatal'))
+
 # Define some variables to help us test that the flang runtime doesn't depend on
 # the C++ runtime libraries. For this we need a C compiler. If for some reason
 # we don't have one, we can just disable the test.
 if config.cc:
     libruntime = os.path.join(config.flang_lib_dir, 'libFortranRuntime.a')
     libdecimal = os.path.join(config.flang_lib_dir, 'libFortranDecimal.a')
     include = os.path.join(config.flang_src_dir, 'include')
 
     if os.path.isfile(libruntime) and os.path.isfile(libdecimal) and os.path.isdir(include):
         config.available_features.add('c-compiler')
         tools.append(ToolSubst('%cc', command=config.cc, unresolved='fatal'))
         tools.append(ToolSubst('%libruntime', command=libruntime,
             unresolved='fatal'))
         tools.append(ToolSubst('%libdecimal', command=libdecimal,
             unresolved='fatal'))
         tools.append(ToolSubst('%include', command=include,
             unresolved='fatal'))
 
+# Add all the tools and their substitutions (if applicable). Use the search paths provided for
+# finding the tools.
 if config.flang_standalone_build:
-    llvm_config.add_tool_substitutions(tools, [config.flang_llvm_tools_dir])
+    llvm_config.add_tool_substitutions(tools, [config.flang_llvm_tools_dir, config.llvm_tools_dir])
 else:
     llvm_config.add_tool_substitutions(tools, config.llvm_tools_dir)
 
 # Enable libpgmath testing
 result = lit_config.params.get("LIBPGMATH")
 if result:
     config.environment["LIBPGMATH"] = True