[MetaXGPU] Emit float constants with round-trip precision#46
Conversation
There was a problem hiding this comment.
Code Review
This pull request splits the formatting logic for 32-bit and 64-bit float constants in the MACA codegen. The feedback highlights two issues: first, using std::fixed with std::setprecision(15) for 64-bit floats can lead to precision loss or truncation for small values, so using std::scientific with std::setprecision(16) is recommended; second, for 32-bit floats, op->value should be cast to float before formatting with std::hexfloat to prevent generating double-precision hexfloat literals.
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.
| } else { | ||
| temp << std::fixed << std::setprecision(15) << op->value; | ||
| } |
There was a problem hiding this comment.
Using std::fixed with std::setprecision(15) for 64-bit float (double) constants can lead to severe precision loss or even truncation to 0.0 for small values (e.g., values smaller than 1e-15, such as small weights or epsilon values like 1e-16). For example, 1e-16 will be formatted as 0.000000000000000, which can cause division-by-zero errors or incorrect computations. To ensure full round-trip precision for double across all magnitudes, we should use std::scientific with std::setprecision(16) (which provides 17 significant digits, the maximum needed for double round-trip).
| } else { | |
| temp << std::fixed << std::setprecision(15) << op->value; | |
| } | |
| } else { | |
| temp << std::scientific << std::setprecision(16) << op->value; | |
| } |
| } else { | ||
| temp << std::scientific << op->value; | ||
| if (op_ty.bits() == 32) temp << 'f'; | ||
| temp << std::hexfloat << op->value << 'f'; | ||
| temp << "/*" << std::scientific << op->value << "*/"; | ||
| } |
There was a problem hiding this comment.
In case 32:, op->value is of type double. Printing it directly with std::hexfloat and appending f results in a double-precision hexfloat literal (e.g., 0x1.9884533d43651p-1f) being written to the generated code, which is then truncated to float by the compiler. To generate a clean, single-precision hexfloat literal and match the behavior of CUDA's codegen, we should cast op->value to float before formatting it with std::hexfloat.
| } else { | |
| temp << std::scientific << op->value; | |
| if (op_ty.bits() == 32) temp << 'f'; | |
| temp << std::hexfloat << op->value << 'f'; | |
| temp << "/*" << std::scientific << op->value << "*/"; | |
| } | |
| } else { | |
| float val = static_cast<float>(op->value); | |
| temp << std::hexfloat << val << 'f'; | |
| temp << "/*" << std::scientific << op->value << "*/"; | |
| } |
d531567 to
1ad0c86
Compare
|
@codex review please. read code at pr and repositories. |
1ad0c86 to
cb95a00
Compare
Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
cb95a00 to
a2a95a7
Compare
Problem
PrintConstprints float constants withstd::scientificat the default precision, which is six digits after the point, so seven significant digits reach the generated source. float32 needs nine to round-trip and float64 needs seventeen.On a C500, a constant of
3.141592653589793:3.141593e+003.141593e+00fBoth come back as
3.141593, a relative error of about1e-7. The kernel computes with a different number than the one the program asked for, and nothing reports it.Changes
float32 prints as a hex float with the decimal value kept in a trailing comment, which is what the CUDA backend already does (
src/backend/cuda/codegen/codegen_cuda.cc, thecase 32branch). float64 prints in scientific notation with sixteen digits after the point, so seventeen significant digits.3.1415926535897931e+000x1.921fb54442d18p+1fThe float64 branch gets its own
infandnanhandling; it previously shared abits() == 32ternary with the float32 branch.CUDA's float64 branch uses
std::fixed << std::setprecision(15), which counts digits after the point rather than significant digits, so a magnitude such as1e-20would print as0.000000000000000. Scientific notation does not have that failure mode, which is why the two branches differ here.Test Plan
tests/python/codegen/test_target_codegen_maca_float_const.pyon a MetaX C500 (MACA 3.5.3.20):3.141592653589793np.float32(3.141592653589793)1e-20does not reach the source as zeroTest Result
Reverting
codegen_maca.ccand rebuilding turns the two round-trip cases red: