microsoft / microsoft/DirectXShaderCompiler

Front-end should widen min-precision stores to full precision during Clang CodeGen

Open
#8,314 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
3.7k
Forks
900
Avg merge
2d 11h
Merged PRs (30d)
44

Description

Summary

When lowering min-precision buffer stores in HLOperationLower.cpp, signedness information (min16int vs min16uint) has been lost — both are just i16 in the IR. This means the widening cast from i16 to i32 must choose between SExt and ZExt without knowing which is correct.

Current Behavior

The store widening in TranslateStore uses SExt unconditionally, which is wrong for min16uint values.

The existing TranslateMinPrecisionRawBuffer pass in DxilGenerationPass.cpp handles this for RawBufferStore by recovering signedness from DxilStructAnnotation / CompType metadata. However, RawBufferVectorStore (SM 6.9+) has no equivalent handling.

Proposed Fix

As suggested by @tex3d in https://github.com/microsoft/DirectXShaderCompiler/pull/8274#discussion_r2996505719:

The front-end during Clang CodeGen should generate the widening cast (to full-precision i32/f32) at the point where the high-level store op is created, so that the store already uses a full-precision type. At that stage, QualType signedness is still available, making it straightforward to emit the correct SExt (signed) or ZExt (unsigned).

Context

  • PR #8274 adds min-precision widening for RawBufferVectorStore and RawBufferStore in HLOperationLower.cpp
  • The SExt/ZExt ambiguity is marked with a TODO referencing this issue

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in HLOperationLower.cpp at TranslateStore and the TODO describing the SExt/ZExt ambiguity, then trace where the high-level store operation is created in Clang CodeGen. Compare the existing signedness recovery in DxilGenerationPass.cpp and the changes from PR #8274. Done means min16int and min16uint stores reach code generation with the correct full-precision widening, including RawBufferVectorStore.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.