Fix constant folding of frac for negative values - #8980
Joel Kiptoo (Kiptoo-Deus) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused implementation matches existing front-end semantics and includes appropriate regression coverage and release notes.
Review effort: Balanced
Findings: None
What changed in this PR
Corrects DXIL constant folding of frac for negative values to match HLSL semantics.
Changes:
- Uses
x - floor(x)when foldingFrc. - Updates and expands negative-matrix regression coverage.
- Documents the fix in release notes.
| File | Description |
|---|---|
lib/Analysis/DxilConstantFolding.cpp |
Corrects Frc folding logic. |
tools/clang/test/HLSLFileCheck/hlsl/intrinsics/basic/frac_literal.hlsl |
Adds corrected regression expectations. |
docs/ReleaseNotes.md |
Records the user-visible bug fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
The DXIL constant folder evaluated Frc as fabs(modf(x)), which is |x - trunc(x)|, so frac(-1.75) folded to 0.75 instead of 0.25. Scalars are folded earlier with x - floor(x), but matrix arguments reach the DXIL folder. Use x - floor(x) there as well. The existing frac_literal.hlsl expectations for the negative matrix elements encoded the old result; update them and add the case from the issue. Fixes microsoft#8937
71dc40d to
74a523f
Compare
|
Joshua Batista (@bob80905) Chris B (@llvm-beanz) sorry for the noise. I had to rebase this onto main because the release notes moved to a new "Upcoming Release" section, and the rebase cleared your approvals. The only change is the release-notes entry moving under the new heading; the fix and tests are the same as what you approved. Could you take another quick look when you have a moment? Thanks! |
Fixes #8937.
DxilConstantFolding.cppfoldedFrcasfabs(modf(x)), which is|x - trunc(x)|, sofrac(-1.75)folded to 0.75 instead of 0.25. Scalars are folded earlier inCGHLSLMSFinishCodeGen.cpp(IOP_frac,v - floor(v)), which is why only matrix arguments showed the problem: they are lowered todx.op.unary(Frc)first and folded by the DXIL folder. This usesx - floor(x)there too, matching the front end andfrac's definition.frac_literal.hlslalready had a negative matrix case, but its expected values were the old result:frac(-21.0002)= 0.0002 andfrac(-0.0012)= 0.0012. Withx - floor(x)these are 0.9998 (0x3FEFFE5C00000000) and 0.9988 (0x3FEFF62B60000000), which I checked independently against float32x - floor(x). I updated those two values and added the matrix from the issue, which now folds to0.25, 0.75, 0.75, 0.25. I also added a release note under Bug Fixes.Testing (macOS arm64, Release build with assertions):
0.25, 0.75, 0.75, 0.25with-HV 202x -T cs_6_0.HLSLFileCheckandCodeGenDXILthat usefracpass.