fix mix MatMul kernels fail with fp64 inputs due to accumulator type …

[wuhulalala]

PR Category

operator

Type of Change

Bug fix

Description

Fix fp64 type mismatch issue in matmul kernels by introducing a dtype-aware accumulator (via IS_FP64) to ensure type consistency during accumulation.

Issue

#2310

Progress

• Change is properly reviewed (1 reviewer required, 2 recommended).
• Change is responded to an issue.
• Change is fully covered by a UT.

Performance

[tengqm]

Thanks for the PR. Please amend the test cases for validation and performance benchmark.

[tengqm]

As a side note ... I'm expecting some failure for this change on some chips because not all backends support FP64. Some test cases for this is really a must.

[wuhulalala]

As a side note ... I'm expecting some failure for this change on some chips because not all backends support FP64. Some test cases for this is really a must.

I will add some FP64 test cases. Thanks for the suggestion.

[wuhulalala]

As a side note ... I'm expecting some failure for this change on some chips because not all backends support FP64. Some test cases for this is really a must.

The choice between FP64 and FP32 is determined at compile time. FP32 is used by default, and FP64 is only enabled when explicitly specified. I will add some FP64 test cases, but I don't think it's necessary to test FP64 on hardware that doesn't support it.

[wuhulalala]

As a side note ... I'm expecting some failure for this change on some chips because not all backends support FP64. Some test cases for this is really a must.

The choice between FP64 and FP32 is determined at compile time. FP32 is used by default, and FP64 is only enabled when explicitly specified. I will add some FP64 test cases, but I don't think it's necessary to test FP64 on hardware that doesn't support it.

I can directly add FP64 support. FlagGems will determine whether the underlying hardware supports FP64 and decide whether to run FP64 tests accordingly.

[tengqm]

As a side note ... I'm expecting some failure for this change on some chips because not all backends support FP64. Some test cases for this is really a must.

The choice between FP64 and FP32 is determined at compile time. FP32 is used by default, and FP64 is only enabled when explicitly specified. I will add some FP64 test cases, but I don't think it's necessary to test FP64 on hardware that doesn't support it.

I can directly add FP64 support. FlagGems will determine whether the underlying hardware supports FP64 and decide whether to run FP64 tests accordingly.

Agreed. We will need to add pytest.mark.skipif.

[wuhulalala]

As a side note ... I'm expecting some failure for this change on some chips because not all backends support FP64. Some test cases for this is really a must.

The choice between FP64 and FP32 is determined at compile time. FP32 is used by default, and FP64 is only enabled when explicitly specified. I will add some FP64 test cases, but I don't think it's necessary to test FP64 on hardware that doesn't support it.

I can directly add FP64 support. FlagGems will determine whether the underlying hardware supports FP64 and decide whether to run FP64 tests accordingly.

Agreed. We will need to add pytest.mark.skipif.
ALL_FLOAT_DTYPES = FLOAT_DTYPES + [torch.float64] if fp64_is_supported else FLOAT_DTYPES
Maybe we can directly use ALL_FLOAT_DTYPES ? it will determine whether the machine supports FP64.

[wuhulalala]

As a side note ... I'm expecting some failure for this change on some chips because not all backends support FP64. Some test cases for this is really a must.

The choice between FP64 and FP32 is determined at compile time. FP32 is used by default, and FP64 is only enabled when explicitly specified. I will add some FP64 test cases, but I don't think it's necessary to test FP64 on hardware that doesn't support it.

I can directly add FP64 support. FlagGems will determine whether the underlying hardware supports FP64 and decide whether to run FP64 tests accordingly.

Agreed. We will need to add pytest.mark.skipif.
ALL_FLOAT_DTYPES = FLOAT_DTYPES + [torch.float64] if fp64_is_supported else FLOAT_DTYPES
Maybe we can directly use ALL_FLOAT_DTYPES ? it will determine whether the machine supports FP64.

fp64_is_supported only reflects the general FP64 capability of the device, but it does not guarantee support for all operators. In particular, tl.dot used in matrix multiplication has stricter constraints and may not support FP64 even when the device does.

To address this, we have refined the test logic by adding more fine-grained skip conditions for mm-related operators, ensuring FP64 tests are only executed when fully supported.
We will add pytest.mark.skipif to handle this properly. You are absolutely right, thanks for pointing this out.

[tengqm]

To address this, we have refined the test logic by adding more fine-grained skip conditions for mm-related operators, ensuring FP64 tests are only executed when fully supported.

Testing is only an approach to help validating that the software works. Passing tests doesn't necessarily mean that software won't break in production environments. We can have many skipif annotations in the test cases so that they won't make noise. However, all those skipif are also bombs that will break users some day.

A more defensive way to do this is to check if the logic is running on the targeted chip, without making assumptions about other backends.