[mlir][llvm] Verify consistency of llvm.resume and llvm.landingpad types

Following the steps of the LLVM verifier
(https://github.com/llvm/llvm-project/blob/b2c48559c882fd558f91e471c4d23ea7b0c6e718/llvm/lib/IR/Verifier.cpp#L4195),
checks in llvm.func verifier are added to ensure consistency of
llvm.landingpad operations' result types and llvm.resume operations'
input types.

As in LLVM, we will allow llvm.resume operations with input values
defined by operations other than llvm.landingpad.

Signed-off-by: Victor Perez <victor.perez@codeplay.com>
Reviewed By: gysit, Dinistro

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

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 428f50f..bfc574a 100644 (file)
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -1263,6 +1263,9 @@ LogicalResult LandingpadOp::verify() {
           "llvm.landingpad needs to be in a function with a personality");
   }
 
+  // Consistency of llvm.landingpad result types is checked in
+  // LLVMFuncOp::verify().
+
   if (!getCleanup() && getOperands().empty())
     return emitError("landingpad instruction expects at least one clause or "
                      "cleanup attribute");
@@ -1523,8 +1526,8 @@ LogicalResult ReturnOp::verify() {
 //===----------------------------------------------------------------------===//
 
 LogicalResult ResumeOp::verify() {
-  if (!getValue().getDefiningOp<LandingpadOp>())
-    return emitOpError("expects landingpad value as operand");
+  // Consistency of llvm.resume value types is checked in LLVMFuncOp::verify().
+
   // No check for personality of function - landingpad op verifies it.
   return success();
 }
@@ -2171,6 +2174,42 @@ LogicalResult LLVMFuncOp::verify() {
     return success();
   }
 
+  Type landingpadResultTy;
+  StringRef diagnosticMessage;
+  bool isLandingpadTypeConsistent =
+      !walk([&](Operation *op) {
+         const auto checkType = [&](Type type, StringRef errorMessage) {
+           if (!landingpadResultTy) {
+             landingpadResultTy = type;
+             return WalkResult::advance();
+           }
+           if (landingpadResultTy != type) {
+             diagnosticMessage = errorMessage;
+             return WalkResult::interrupt();
+           }
+           return WalkResult::advance();
+         };
+         return TypeSwitch<Operation *, WalkResult>(op)
+             .Case<LandingpadOp>([&](auto landingpad) {
+               constexpr StringLiteral errorMessage =
+                   "'llvm.landingpad' should have a consistent result type "
+                   "inside a function";
+               return checkType(landingpad.getType(), errorMessage);
+             })
+             .Case<ResumeOp>([&](auto resume) {
+               constexpr StringLiteral errorMessage =
+                   "'llvm.resume' should have a consistent input type inside a "
+                   "function";
+               return checkType(resume.getValue().getType(), errorMessage);
+             })
+             .Default([](auto) { return WalkResult::skip(); });
+       }).wasInterrupted();
+  if (!isLandingpadTypeConsistent) {
+    assert(!diagnosticMessage.empty() &&
+           "Expecting a non-empty diagnostic message");
+    return emitError(diagnosticMessage);
+  }
+
   return success();
 }
 

diff --git a/mlir/test/Dialect/LLVMIR/invalid-typed-pointers.mlir b/mlir/test/Dialect/LLVMIR/invalid-typed-pointers.mlir
index 033b84d..c57a37d 100644 (file)
--- a/mlir/test/Dialect/LLVMIR/invalid-typed-pointers.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid-typed-pointers.mlir
@@ -138,15 +138,39 @@ llvm.func @caller(%arg0: i32) -> i32 attributes { personality = @__gxx_personali
 llvm.func @foo(i32) -> i32
 llvm.func @__gxx_personality_v0(...) -> i32
 
+// expected-error@below {{'llvm.resume' should have a consistent input type inside a function}}
+llvm.func @caller(%arg0: i32, %arg1: !llvm.struct<(ptr<i32>, i32)>) -> i32 attributes { personality = @__gxx_personality_v0 } {
+  %0 = llvm.invoke @foo(%arg0) to ^bb1 unwind ^bb2 : (i32) -> i32
+^bb1:
+  %1 = llvm.invoke @foo(%0) to ^bb3 unwind ^bb4 : (i32) -> i32
+^bb2:
+  %2 = llvm.landingpad cleanup : !llvm.struct<(ptr<i8>, i32)>
+  llvm.resume %arg1 : !llvm.struct<(ptr<i32>, i32)>
+^bb3:
+  llvm.return %1 : i32
+^bb4:
+  %3 = llvm.landingpad cleanup : !llvm.struct<(ptr<i8>, i32)>
+  llvm.resume %3 : !llvm.struct<(ptr<i8>, i32)>
+}
+
+// -----
+
+llvm.func @foo(i32) -> i32
+llvm.func @__gxx_personality_v0(...) -> i32
+
+// expected-error@below {{'llvm.landingpad' should have a consistent result type inside a function}}
 llvm.func @caller(%arg0: i32) -> i32 attributes { personality = @__gxx_personality_v0 } {
-  %0 = llvm.mlir.constant(1 : i32) : i32
-  %1 = llvm.invoke @foo(%0) to ^bb1 unwind ^bb2 : (i32) -> i32
-^bb1: // pred: ^bb0
-  llvm.return %0 : i32
-^bb2: // pred: ^bb0
+  %0 = llvm.invoke @foo(%arg0) to ^bb1 unwind ^bb2 : (i32) -> i32
+^bb1:
+  %1 = llvm.invoke @foo(%0) to ^bb3 unwind ^bb4 : (i32) -> i32
+^bb2:
   %2 = llvm.landingpad cleanup : !llvm.struct<(ptr<i8>, i32)>
-  // expected-error@+1 {{'llvm.resume' op expects landingpad value as operand}}
-  llvm.resume %0 : i32
+  llvm.resume %2 : !llvm.struct<(ptr<i8>, i32)>
+^bb3:
+  llvm.return %1 : i32
+^bb4:
+  %3 = llvm.landingpad cleanup : !llvm.struct<(ptr<i32>, i32)>
+  llvm.resume %3 : !llvm.struct<(ptr<i32>, i32)>
 }
 
 // -----

diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index c3af84e..aa3498a 100644 (file)
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -791,15 +791,39 @@ llvm.func @caller(%arg0: i32) -> i32 attributes { personality = @__gxx_personali
 llvm.func @foo(i32) -> i32
 llvm.func @__gxx_personality_v0(...) -> i32
 
+// expected-error@below {{'llvm.resume' should have a consistent input type inside a function}}
 llvm.func @caller(%arg0: i32) -> i32 attributes { personality = @__gxx_personality_v0 } {
-  %0 = llvm.mlir.constant(1 : i32) : i32
-  %1 = llvm.invoke @foo(%0) to ^bb1 unwind ^bb2 : (i32) -> i32
-^bb1: // pred: ^bb0
-  llvm.return %0 : i32
-^bb2: // pred: ^bb0
+  %0 = llvm.invoke @foo(%arg0) to ^bb1 unwind ^bb2 : (i32) -> i32
+^bb1:
+  %1 = llvm.invoke @foo(%0) to ^bb3 unwind ^bb4 : (i32) -> i32
+^bb2:
   %2 = llvm.landingpad cleanup : !llvm.struct<(ptr, i32)>
-  // expected-error@+1 {{'llvm.resume' op expects landingpad value as operand}}
   llvm.resume %0 : i32
+^bb3:
+  llvm.return %1 : i32
+^bb4:
+  %3 = llvm.landingpad cleanup : !llvm.struct<(ptr, i32)>
+  llvm.resume %3 : !llvm.struct<(ptr, i32)>
+}
+
+// -----
+
+llvm.func @foo(i32) -> i32
+llvm.func @__gxx_personality_v0(...) -> i32
+
+// expected-error@below {{'llvm.landingpad' should have a consistent result type inside a function}}
+llvm.func @caller(%arg0: i32) -> i32 attributes { personality = @__gxx_personality_v0 } {
+  %0 = llvm.invoke @foo(%arg0) to ^bb1 unwind ^bb2 : (i32) -> i32
+^bb1:
+  %1 = llvm.invoke @foo(%0) to ^bb3 unwind ^bb4 : (i32) -> i32
+^bb2:
+  %2 = llvm.landingpad cleanup : !llvm.struct<(ptr, i32)>
+  llvm.resume %2 : !llvm.struct<(ptr, i32)>
+^bb3:
+  llvm.return %1 : i32
+^bb4:
+  %3 = llvm.landingpad cleanup : i32
+  llvm.resume %3 : i32
 }
 
 // -----