[MetaXGPU] Split the fast-math and precise lowering paths, as CUDA does#45
[MetaXGPU] Split the fast-math and precise lowering paths, as CUDA does#45Lfan-ke wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request separates the lowering paths for several mathematical intrinsics (such as exp, log, tan, cos, and sin) into standard and fast-math variants. The reviewer pointed out that using MACAFastMathTan for the fast-math path of tirx.tan breaks bfloat16 support because it lacks support for kDLBfloat, and suggested using MACAMath for both paths instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| TVM_REGISTER_OP("tirx.tan") | ||
| .set_attr<FLowerIntrinsic>("maca.fastmath.FLowerIntrinsic", | ||
| DispatchPureExtern<MACAFastMathTan>) | ||
| .set_attr<FLowerIntrinsic>("maca.FLowerIntrinsic", DispatchPureExtern<MACAMath>); |
There was a problem hiding this comment.
The MACAFastMathTan struct is functionally identical to MACAMath for float types (since it avoids __tanf and uses tanf instead), but it lacks support for other types like kDLBfloat (returning "" instead of "htan"). This breaks bfloat16 support for tan when fast-math is enabled.
We can simplify this registration and fix the bfloat16 bug by using MACAMath for both the default and fast-math paths. Additionally, MACAFastMathTan (lines 83-101) can be completely removed from this file as it is no longer needed.
| TVM_REGISTER_OP("tirx.tan") | |
| .set_attr<FLowerIntrinsic>("maca.fastmath.FLowerIntrinsic", | |
| DispatchPureExtern<MACAFastMathTan>) | |
| .set_attr<FLowerIntrinsic>("maca.FLowerIntrinsic", DispatchPureExtern<MACAMath>); | |
| TVM_REGISTER_OP("tirx.tan") | |
| .set_attr<FLowerIntrinsic>("maca.fastmath.FLowerIntrinsic", | |
| DispatchPureExtern<MACAMath>) | |
| .set_attr<FLowerIntrinsic>("maca.FLowerIntrinsic", DispatchPureExtern<MACAMath>); |
|
@codex review please. read code at pr and repositories. |
bcaa6e9 to
7d30c04
Compare
7d30c04 to
e382dcc
Compare
Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
e382dcc to
a5b6a93
Compare
Problem
tirx.enable_fast_mathdefaults to false, andIntrinInjecteronly consults<target>.fastmath.FLowerIntrinsicwhen it is set (src/tirx/transform/lower_intrin.cc:57-60). The MACA rules register their fast-math variants under the plainmaca.FLowerIntrinsickey instead, so the flag never reaches them: seven ops lower to the approximate device function unconditionally.On a C500, with the default
PassContext:exp__expfexpfexp10__exp10fexp10flog__logflogflog2__log2flog2flog10__log10flog10fsin__sinfsinfcos__cosfcosfA program that never asked for fast math gets it anyway, and setting the flag changes nothing. The CUDA rules do not have this problem:
src/backend/cuda/codegen/intrin_rule_cuda.cccarries tencuda.fastmath.FLowerIntrinsicregistrations alongside the precise ones.Changes
Each of these ops now registers twice, the way CUDA does:
MACAFastMathonmaca.fastmath.FLowerIntrinsic,MACAMathonmaca.FLowerIntrinsic. The default becomes precise andtirx.enable_fast_mathbecomes the thing that selects the approximation.powgains a fast-math lowering it did not have before;__powfcompiles on a C500.tankeepsMACAFastMathTanon the fast path, which yieldstanfat fp32 — the same function the precise path emits — so the registration only affects it at other widths.tanhstays precise on both paths, because MACA has no__tanhfand mxcc rejects the symbol.Test Plan
tests/python/codegen/test_target_codegen_maca_fastmath.pyon a MetaX C500 (MACA 3.5.3.20), over the eight ops that have both forms:PassContext, the generated source callsexpfand never__expfPassContext(config={"tirx.enable_fast_math": True}), it calls__expfTest Result
Reverting
intrin_rule_maca.ccand rebuilding turns eight of them red — the seven ops above lose their precise default, andpowloses the opt-in path: