[Flang] Replace notifyMatchFailure with TODO hard failures

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.

Reviewed By: clementval, awarzynski

Differential Revision: https://reviews.llvm.org/D114371

flang/lib/Optimizer/CodeGen/CodeGen.cpp

@@ -541,8 +541,8 @@ struct BoxProcHostOpConversion : public FIROpConversion<fir::BoxProcHostOp> {
   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();
   }
 };
 
@@ -783,8 +783,8 @@ struct DispatchOpConversion : public FIROpConversion<fir::DispatchOp> {
   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();
   }
 };
 
@@ -797,8 +797,8 @@ struct DispatchTableOpConversion
   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();
   }
 };
 
@@ -810,8 +810,8 @@ struct DTEntryOpConversion : public FIROpConversion<fir::DTEntryOp> {
   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();
   }
 };
 
@@ -822,8 +822,8 @@ struct GlobalLenOpConversion : public FIROpConversion<fir::GlobalLenOp> {
   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();
   }
 };
 
@@ -836,8 +836,7 @@ struct LenParamIndexOpConversion
   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");
   }
 };
 
@@ -848,8 +847,8 @@ struct GenTypeDescOpConversion : public FIROpConversion<fir::GenTypeDescOp> {
   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();
   }
 };
 
@@ -860,8 +859,8 @@ struct FirEndOpConversion : public FIROpConversion<fir::FirEndOp> {
   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();
   }
 };
 
@@ -1021,11 +1020,11 @@ struct SelectCaseOpConversion : public FIROpConversion<fir::SelectCaseOp> {
     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) {
@@ -1182,8 +1181,9 @@ struct SelectTypeOpConversion : public FIROpConversion<fir::SelectTypeOp> {
   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();
   }
 };
 
@@ -1254,7 +1254,7 @@ struct ZeroOpConversion : public FIROpConversion<fir::ZeroOp> {
   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>()) {
@@ -1575,10 +1575,11 @@ struct EmboxOpConversion : public EmboxCommonConversion<fir::EmboxOp> {
                              /*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();
@@ -1593,12 +1594,11 @@ struct EmboxProcOpConversion : public FIROpConversion<fir::EmboxProcOp> {
   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
@@ -2110,8 +2110,8 @@ struct UnboxProcOpConversion : public FIROpConversion<fir::UnboxProcOp> {
   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();
   }
 };
 

flang/test/CMakeLists.txt

@@ -46,7 +46,7 @@ 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)

flang/test/Fir/Todo/boxproc_host.fir

@@ -0,0 +1,10 @@
+// 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

@@ -0,0 +1,10 @@
+// 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

@@ -0,0 +1,9 @@
+// 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

@@ -0,0 +1,11 @@
+// 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

@@ -0,0 +1,9 @@
+// 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

@@ -0,0 +1,10 @@
+// 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

@@ -0,0 +1,11 @@
+// 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

@@ -0,0 +1,11 @@
+// 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

@@ -0,0 +1,19 @@
+// 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

@@ -0,0 +1,11 @@
+// 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

@@ -13,99 +13,6 @@ func @zero_aggregate() {
 
 // -----
 
-// 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'}}
@@ -161,32 +68,23 @@ func @shape_shift_not_dead(%arg0: !fir.ref<!fir.array<?x?xf32>>, %i: index, %j:
 
 // -----
 
-// Test that the type `fir.boxproc` fails to be legalized.
-// Not implemented yet.
+// Test `fir.select_type` conversion to llvm.
+// Should have been converted.
 
-// 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.
-
-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

@@ -35,6 +35,10 @@ 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.
@@ -72,6 +76,15 @@ tools = [
     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.
@@ -90,8 +103,10 @@ if config.cc:
         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)