Skip to content

Commit bd16df2

Browse files
[TorchConversion] Reject non-shaped function signatures in backend-co… (#4728)
The backend-contract verifiers check `func.func` legality with `converter.isLegal(op)`, but that only looks at an op's operand/result values. A function's argument and result types live in its `FunctionType` attribute, which never gets inspected this way. So a function argument of a non-shaped type that happens to be dead (referenced by nothing in the body) sails through the verifier. We hit this with an ONNX model whose graph input is an optional. The importer preserves it as a `!torch.optional`argument, `OptionalHasElement` constant-folds, and the argument is left dead on the signature. The verifier reported success, and the illegal type then blew up a downstream consumer instead of being caught here where it should be. Tightening the verifier to also check the signature via `isSignatureLegal` seems like the right place to stop this — it turns a confusing downstream crash into the verifier's existing contract diagnostic. Same gap and fix apply to all three verifiers (linalg, TOSA, Stablehlo), so I've fixed them together and added a regression test per backend. One existing TOSA pipeline test had a dead `!torch.float` argument on its signature and only passed because of this gap. TOSA operates on tensors, not loose scalar primitives — had that scalar been used in the body the verifier would already have rejected it. So the test was locking down IR that isn't really legal; I dropped the unused argument. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1deff3a commit bd16df2

8 files changed

Lines changed: 98 additions & 9 deletions

lib/Dialect/TorchConversion/Transforms/VerifyLinalgOnTensorsBackendContract.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ class VerifyLinalgOnTensorsBackendContractPass
5656
}
5757

5858
auto opHasLegalTypes = [&](Operation *op) { return converter.isLegal(op); };
59+
// `converter.isLegal(op)` inspects only operands/results; a `func.func`'s
60+
// signature is in its `FunctionType` attribute and would otherwise go
61+
// unchecked (e.g. an unused non-shaped arg like `!torch.optional`).
62+
auto funcHasLegalTypes = [&](func::FuncOp func) {
63+
return converter.isSignatureLegal(func.getFunctionType());
64+
};
5965
auto isLegalScalarOp = [&](Operation *op) {
6066
// We recognize basic scalar ops by them having the trait "Elementwise",
6167
// even though we don't expect them to operate on tensors.
@@ -66,8 +72,9 @@ class VerifyLinalgOnTensorsBackendContractPass
6672
ConversionTarget target(*context);
6773

6874
// Structural operations.
69-
target.addDynamicallyLegalOp<ModuleOp, func::FuncOp, func::ReturnOp>(
70-
opHasLegalTypes);
75+
target.addDynamicallyLegalOp<func::FuncOp>(funcHasLegalTypes);
76+
target.addDynamicallyLegalOp<func::ReturnOp>(opHasLegalTypes);
77+
target.addLegalOp<ModuleOp>();
7178

7279
target.addDynamicallyLegalOp<GetNextSeedOp>(opHasLegalTypes);
7380

lib/Dialect/TorchConversion/Transforms/VerifyStablehloBackendContract.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,20 @@ class VerifyStablehloBackendContractPass
4242
});
4343

4444
auto opHasLegalTypes = [&](Operation *op) { return converter.isLegal(op); };
45+
// `converter.isLegal(op)` inspects only operands/results; a `func.func`'s
46+
// signature is in its `FunctionType` attribute and would otherwise go
47+
// unchecked (e.g. an unused non-shaped arg like `!torch.optional`).
48+
auto funcHasLegalTypes = [&](func::FuncOp func) {
49+
return converter.isSignatureLegal(func.getFunctionType());
50+
};
4551

4652
MLIRContext *context = &getContext();
4753
ConversionTarget target(*context);
4854

4955
// Structural operations.
50-
target.addDynamicallyLegalOp<ModuleOp, func::FuncOp, func::ReturnOp>(
51-
opHasLegalTypes);
56+
target.addDynamicallyLegalOp<func::FuncOp>(funcHasLegalTypes);
57+
target.addDynamicallyLegalOp<func::ReturnOp>(opHasLegalTypes);
58+
target.addLegalOp<ModuleOp>();
5259

5360
target.addLegalDialect<chlo::ChloDialect>();
5461
target.addLegalDialect<stablehlo::StablehloDialect>();

lib/Dialect/TorchConversion/Transforms/VerifyTosaBackendContract.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,19 @@ class VerifyTosaBackendContractPass
3838
});
3939

4040
auto opHasLegalTypes = [&](Operation *op) { return converter.isLegal(op); };
41+
// `converter.isLegal(op)` inspects only operands/results; a `func.func`'s
42+
// signature is in its `FunctionType` attribute and would otherwise go
43+
// unchecked (e.g. an unused non-shaped arg like `!torch.optional`).
44+
auto funcHasLegalTypes = [&](func::FuncOp func) {
45+
return converter.isSignatureLegal(func.getFunctionType());
46+
};
4147

4248
ConversionTarget target(*context);
4349

4450
// Structural operations.
45-
target.addDynamicallyLegalOp<ModuleOp, func::FuncOp, func::ReturnOp>(
46-
opHasLegalTypes);
51+
target.addDynamicallyLegalOp<func::FuncOp>(funcHasLegalTypes);
52+
target.addDynamicallyLegalOp<func::ReturnOp>(opHasLegalTypes);
53+
target.addLegalOp<ModuleOp>();
4754
// Basic scalar operations.
4855
target.addLegalDialect<tosa::TosaDialect>();
4956
target.addDynamicallyLegalOp<tensor::CastOp>(opHasLegalTypes);

test/Conversion/TorchToTosa/torch-backend-to-tosa-backend-pipeline.mlir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func.func @torch.aten.mul.Scalar$mixed_type(%arg0: !torch.vtensor<[5],bf16>) ->
2020
// CHECK-SAME: %[[VAL_1:.*]]: tensor<6xf32>
2121
// CHECK: %[[VAL_3:.*]] = tosa.cast %[[VAL_1]] : (tensor<6xf32>) -> tensor<6xbf16>
2222
// CHECK: %[[VAL_4:.*]] = tosa.add %[[VAL_0]], %[[VAL_3]] : (tensor<6xbf16>, tensor<6xbf16>) -> tensor<6xbf16>
23-
func.func @torch.aten.add.Tensor$mixed_type_fp(%arg0: !torch.vtensor<[6],bf16>, %arg1: !torch.vtensor<[6],f32>, %arg2: !torch.float) -> !torch.vtensor<[6],bf16> {
23+
func.func @torch.aten.add.Tensor$mixed_type_fp(%arg0: !torch.vtensor<[6],bf16>, %arg1: !torch.vtensor<[6],f32>) -> !torch.vtensor<[6],bf16> {
2424
%float1 = torch.constant.float 1.000000e+00
2525
%0 = torch.aten.add.Tensor %arg0, %arg1, %float1 : !torch.vtensor<[6],bf16>, !torch.vtensor<[6],f32>, !torch.float -> !torch.vtensor<[6],bf16>
2626
return %0 : !torch.vtensor<[6],bf16>

test/Dialect/TorchConversion/verify-linalg-on-tensors-backend-contract.mlir

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,22 @@ module {
4646

4747
// expected-error@+1 {{Module does not conform to the linalg-on-tensors backend contract.}}
4848
module {
49+
// expected-error@+1 {{failed to legalize operation 'func.func'}}
4950
func.func @disallowed(%arg0: !torch.tensor) -> !torch.tensor {
50-
// expected-error@+1 {{failed to legalize operation 'func.return'}}
5151
return %arg0 : !torch.tensor
5252
}
5353
}
54+
55+
// -----
56+
57+
// A non-shaped function argument (e.g. `!torch.optional`) that is dead --
58+
// referenced by no op in the body -- should be rejected.
59+
60+
// expected-error@+1 {{Module does not conform to the linalg-on-tensors backend contract.}}
61+
module {
62+
// expected-error@+1 {{failed to legalize operation 'func.func'}}
63+
func.func @dead_optional_arg(%arg0: !torch.optional<vtensor<[4],f32>>) -> tensor<1xi64> {
64+
%cst = arith.constant dense<1> : tensor<1xi64>
65+
return %cst : tensor<1xi64>
66+
}
67+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: torch-mlir-opt -torch-verify-stablehlo-backend-contract -split-input-file -verify-diagnostics -allow-unregistered-dialect %s | FileCheck %s
2+
// REQUIRES: stablehlo
3+
4+
// CHECK: func.func @tanh
5+
func.func @tanh(%arg0: tensor<?x?xf32>) -> tensor<?x?xf32> {
6+
%0 = stablehlo.tanh %arg0 : tensor<?x?xf32>
7+
return %0 : tensor<?x?xf32>
8+
}
9+
10+
// -----
11+
12+
// Basic check of error reporting.
13+
14+
// expected-error@+1 {{Module does not conform to the Stablehlo backend contract.}}
15+
module {
16+
func.func @disallowed() {
17+
// expected-error@+1 {{failed to legalize operation 'unknown_dialect.unknown_op'}}
18+
"unknown_dialect.unknown_op"() : () -> ()
19+
return
20+
}
21+
}
22+
23+
// -----
24+
25+
// A non-shaped function argument (e.g. `!torch.optional`) that is dead --
26+
// referenced by no op in the body -- should be rejected.
27+
28+
// expected-error@+1 {{Module does not conform to the Stablehlo backend contract.}}
29+
module {
30+
// expected-error@+1 {{failed to legalize operation 'func.func'}}
31+
func.func @dead_optional_arg(%arg0: !torch.optional<vtensor<[4],f32>>) -> tensor<1xi64> {
32+
%cst = arith.constant dense<1> : tensor<1xi64>
33+
return %cst : tensor<1xi64>
34+
}
35+
}

test/Dialect/TorchConversion/verify-tosa-backend-contract.mlir

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,22 @@ module {
3535

3636
// expected-error@+1 {{Module does not conform to the TOSA backend contract.}}
3737
module {
38+
// expected-error@+1 {{failed to legalize operation 'func.func'}}
3839
func.func @disallowed(%arg0: !torch.tensor) -> !torch.tensor {
39-
// expected-error@+1 {{failed to legalize operation 'func.return'}}
4040
return %arg0 : !torch.tensor
4141
}
4242
}
43+
44+
// -----
45+
46+
// A non-shaped function argument (e.g. `!torch.optional`) that is dead --
47+
// referenced by no op in the body -- should be rejected.
48+
49+
// expected-error@+1 {{Module does not conform to the TOSA backend contract.}}
50+
module {
51+
// expected-error@+1 {{failed to legalize operation 'func.func'}}
52+
func.func @dead_optional_arg(%arg0: !torch.optional<vtensor<[4],f32>>) -> tensor<1xi64> {
53+
%cst = arith.constant dense<1> : tensor<1xi64>
54+
return %cst : tensor<1xi64>
55+
}
56+
}

test/lit.cfg.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@
3535
config.substitutions.append(("%PATH%", config.environment["PATH"]))
3636
config.substitutions.append(("%shlibext", config.llvm_shlib_ext))
3737

38+
# Register optional-backend availability as lit features so tests that exercise a
39+
# specific backend contract can guard themselves with `// REQUIRES: <backend>`.
40+
if getattr(config, "enable_stablehlo", False):
41+
config.available_features.add("stablehlo")
42+
3843
llvm_config.with_system_environment(["HOME", "INCLUDE", "LIB", "TMP", "TEMP"])
3944

4045
# llvm_config.use_default_substitutions()

0 commit comments

Comments
 (0)