Skip to content

Commit 9d66dca

Browse files
authored
[mlir][tosa] Work around GCC bug in tosa-to-tensor (#91521)
GCC 12 and 13 generate incorrect code for a pattern in the tosa-to-tensor pass responsible for lowering tosa.reshape. This results in the tosa.reshape lowering producing IR which fails to verify. I've narrowed down the set of cmake flags needed to reproduce the issue to this: cmake -G Ninja ../llvm \ -DLLVM_ENABLE_PROJECTS="mlir" \ -DLLVM_TARGETS_TO_BUILD=host \ -DLLVM_ENABLE_PROJECTS=mlir \ -DCMAKE_BUILD_TYPE="Release" \ -DCMAKE_CXX_FLAGS_RELEASE="-O2" \ -DCMAKE_CXX_FLAGS="-O2" \ -DCMAKE_CXX_COMPILER=g++ \ -DCMAKE_C_COMPILER=gcc This is the failing test case: func.func @fails_in_gcc_12(%arg0: tensor<?xf32>) -> tensor<1x1x1x?xf32> { %0 = tosa.reshape %arg0 {new_shape = array<i64: 1, 1, 1, -1>} : (tensor<?xf32>) -> tensor<1x1x1x?xf32> return %0 : tensor<1x1x1x?xf32> } This should lower to a tensor.expand_shape operation like so: func.func @foo(%arg0: tensor<?xf32>) -> tensor<1x1x1x?xf32> { %c0 = arith.constant 0 : index %dim = tensor.dim %arg0, %c0 : tensor<?xf32> %c1 = arith.constant 1 : index %expanded = tensor.expand_shape %arg0 [[0, 1, 2, 3]] output_shape [1, 1, 1, %dim] : tensor<?xf32> into tensor<1x1x1x?xf32> return %expanded : tensor<1x1x1x?xf32> } Under GCC 12/13 with the above cmake configuration, the tensor.expand_shape looks like this %2 = "tensor.expand_shape"(%arg0) <{reassociation = [[0, 1, 2, 3]], static_output_shape = array<i64>}> : (tensor<?xf32>) -> tensor<?x1x1x?xf32> The key difference is the computed output type of `tensor<?x1x1x?xf32>` rather than the expected `tensor<1x1x1x?xf32>`. This expand_shape fails to verify with this error message: error: 'tensor.expand_shape' op expected number of static shape dims to be equal to the output rank (4) but found 0 inputs instead The problematic code is calculating the intermediate shape of the generated tensor.expand_shape operation in the expand_shape/collapse_shape sequence that implements tosa.reshape. // Compute result shape bool resultIsStatic = true; auto resultShape = llvm::map_to_vector(newShape, [&](int64_t size) { // Omitted // If we do not know the total size of the tensor, keep this dimension // dynamic in the result shape. if (!inputIsStatic) { resultIsStatic = false; return ShapedType::kDynamic; } }); if (resultIsStatic) { // do something return; } // do something else return; The failure point seems to be the update of the resultIsStatic variable in the lambda body. The assignment of false is not propagated to the use in the if-statement, resulting in the branch being taken when it should not. I've found several modification to the code that gets around the bug. The version I settled on is one which makes the logic a little more obvious.
1 parent 31774b6 commit 9d66dca

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,17 @@ TensorType inferReshapeExpandedType(TensorType inputType,
5555
// Check if the input is static, and if so, get its total size
5656
bool inputIsStatic = inputType.hasStaticShape();
5757
int64_t totalSize = inputIsStatic ? inputType.getNumElements() : -1;
58-
58+
5959
// Compute result shape
60-
bool resultIsStatic = true;
6160
auto resultShape = llvm::map_to_vector(newShape, [&](int64_t size) -> int64_t {
6261
// If this is not a placeholder, do not change it
6362
if (size >= 0)
6463
return size;
6564

6665
// If we do not know the total size of the tensor, keep this dimension
6766
// dynamic in the result shape.
68-
if (!inputIsStatic) {
69-
resultIsStatic = false;
67+
if (!inputIsStatic)
7068
return ShapedType::kDynamic;
71-
}
7269

7370
// Calculate the product of all elements in 'newShape' except for the -1
7471
// placeholder, which we discard by negating the result.
@@ -84,12 +81,14 @@ TensorType inferReshapeExpandedType(TensorType inputType,
8481
return totalSize / totalSizeNoPlaceholder;
8582
});
8683

84+
bool resultIsStatic = !ShapedType::isDynamicShape(resultShape);
85+
8786
// A syntactic restriction in 'tensor.expand_shape' forbids a dynamically
8887
// shaped input from being reshaped into a statically shaped result. We may
8988
// simply turn the first result dimension dynamic to address this.
9089
if (!inputIsStatic && resultIsStatic)
9190
resultShape[0] = ShapedType::kDynamic;
92-
91+
9392
// The 'tensor.expand_shape' op also forbids a statically shaped input from
9493
// being reshaped into a dynamically shaped result, but the placeholder
9594
// inference algorithm above guarantees that this will never be the case.

mlir/test/Conversion/TosaToTensor/tosa-to-tensor.mlir

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,21 @@ func.func @test_reshape_6d_down_s2s_auto(%arg0: tensor<1x2x3x5x7x11xf32>) -> ten
394394

395395
// -----
396396

397+
// This test would previously fail on GCC with certain compiler flags.
398+
// The GCC issue would cause invalid IR after tosa-to-tensor, so this test
399+
// locks down that the code goes through tosa-to-tensor and verifies.
400+
//
401+
// See https://github.com/llvm/llvm-project/pull/91521 for a full description.
402+
403+
// CHECK-LABEL: reshape_bug_fix
404+
// CHECK: tensor.expand_shape
405+
func.func @reshape_bug_fix(%arg0: tensor<?xf32>) -> tensor<1x1x1x?xf32> {
406+
%0 = tosa.reshape %arg0 {new_shape = array<i64: 1, 1, 1, -1>} : (tensor<?xf32>) -> tensor<1x1x1x?xf32>
407+
return %0 : tensor<1x1x1x?xf32>
408+
}
409+
410+
// -----
411+
397412
// CHECK-LABEL: test_reshape_6d_down_s2s_explicit
398413
// CHECK-SAME: %[[ARG_0:[a-zA-Z0-9_]+]]: tensor<1x2x3x5x7x11xf32>
399414
// CHECK: %[[VAL_0:.*]] = tensor.collapse_shape %[[ARG_0]] {{\[\[}}0, 1, 2], [3], [4, 5]] : tensor<1x2x3x5x7x11xf32> into tensor<6x5x77xf32>

0 commit comments

Comments
 (0)