Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Improve capacity of folding constant values at parse-time.

Merged Francisco Casas requested to merge fcasas/vkd3d:default_values_part1 into master
3 unresolved threads

This is basically the first 6 commits of the current version of !787 (merged).

The previous approach to folding default values into constants was to hold an hlsl_src to the instruction in struct hlsl_default_value and, after applying most of the passes in hlsl_emit_bytecode() check if those were successfully lowered to a single hlsl_ir_constant instruction.

@zfigura suggested we try to lower these values to constants at parse time instead, and I think this was a good call.

It requires calling more passes at parse time though, but I think that is good because these are also required in other places like array sizes.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1 [pixel shader todo]
2 float2 a = {1, 2};
3 float4x2 b = {1, 2, 3, 4, 5, 6, 7, 8};
  • Comment on lines +2 to +3

    How is this feature supposed to work? Does the variable take the default value is no CB is provided at run time? Is the default also used if a CB is provided but it's too short (for the variables past the CB end)?

    I think that we should always run non-failing shaders, even more here that we're testing a new (new for us, at least) feature. Is there anything that makes this difficult?

  • If I understand the documentation correctly, default values can only be accessed through reflection and then manually copied into the CB. So they have to be written in the right place in the tpf and d3dbc binary.

    Global variables that are not marked static or extern are not compiled into the shader. The compiler does not automatically set default values for global variables and cannot use them in optimizations. To initialize this type of global variable, use reflection to get its value and then copy the value to a constant buffer. For example, you can use the ID3D11ShaderReflection::GetVariableByName method to get the variable, use the ID3D11ShaderReflectionVariable::GetDesc method to get the shader-variable description, and get the initial value from the DefaultValue member of the D3D11_SHADER_VARIABLE_DESC structure...

    But it also can be used automatically for the effects framework:

    You can also use the effects framework to automatically process the reflecting and setting the initial value. For example, you can use the ID3DX11EffectPass::Apply method.


    I think that we should always run non-failing shaders, even more here that we're testing a new (new for us, at least) feature. Is there anything that makes this difficult?

    I thought it wasn't so necessary here because what we really want to test is that the default values are correctly parsed.

    We can't test if they are properly written (in fact they are not yet) because the shader_runner doesn't have that capability. OTOH, Zeb implemented default values reflection and I introduced e6fad279 in !787 (merged) in order to test that, at least for SM4.

    I will add the [test] blocks then.

  • If I understand the documentation correctly, default values can only be accessed through reflection and then manually copied into the CB. So they have to be written in the right place in the tpf and d3dbc binary.

    Ok, thanks for the explanation.

  • On the other hand, since we want to make sure we're parsing values correctly, we should then have reflection tests for these things.

  • Please register or sign in to reply
  • added 6 commits

    • c32db64d - tests: Test default values for uniform variables.
    • 6d86f7d4 - tests: Test default values for constant buffer variables.
    • f6ec66e4 - vkd3d-shader/hlsl: Run constant passes in a separate function.
    • 1b736585 - vkd3d-shader/hlsl: Run more constant passes on static expressions eval.
    • 24dc3b60 - tests: Test complex array size expression.
    • 30e76816 - vkd3d-shader/hlsl: Also lower matrix swizzles and index loads in const passes.

    Compare with previous version

    • The following test is failing on Windows:

      float2 a = {1, 2};
      float4x2 b = {1, 2, 3, 4, 5, 6, 7, 8};
      
      float4 main() : sv_target
      {
          return float4(a, b[2]);
      }

      not because of default values, but because native allocates b before a:

      // Parameters:
      //
      //   float2 a;
      //   float4x2 b;
      //
      //
      // Registers:
      //
      //   Name         Reg   Size
      //   ------------ ----- ----
      //   b            c0       2
      //   a            c2       1
      //
      
          ps_2_0
          mov r0.xy, c2
          mov r0.z, c0.z
          mov r0.w, c1.z
          mov oC0, r0

      This means that variables are not allocated in declaration order.

    • Hmm, I don't know why that's happening, but I wouldn't allow this to block the MR. I guess you can force CB allocation on SM1-3 too, right? If so just do it and leave the problem of register allocation for another MR.

    • Yes, I can use register reservations to force the order but I preferred to specify the different uniform offsets for SM1 and SM4 and leave the probe directive as todo(sm<4), to not forget that the bug is still present.

    • I vaguely recall that sm1 variables are allocated in order of size.

    • Yep, I am working on fixing this.

    • For now I left the line as

      todo(sm<4) probe all rgba (10, 20, 50, 60)

      this should be solved by !851 (merged).

    • Please register or sign in to reply
  • Giovanni Mascellani
    Giovanni Mascellani @giomasce started a thread on an outdated change in commit c32db64d
  • 2 % The default values are intended to be obtained using reflection.
    3
    4 [pixel shader todo]
    5 float2 a = {1, 2};
    6 float4x2 b = {1, 2, 3, 4, 5, 6, 7, 8};
    7
    8 float4 main() : sv_target
    9 {
    10 return float4(a, b[2]);
    11 }
    12
    13 [test]
    14 uniform 0 float4 10 20 0 0
    15 uniform 4 float4 10 30 50 70
    16 uniform 8 float4 20 40 60 80
    17 todo(sm<6 | glsl) draw quad
  • Giovanni Mascellani approved this merge request

    approved this merge request

  • added 6 commits

    • 1bb3c393 - tests: Test default values for uniform variables.
    • b4186aaa - tests: Test default values for constant buffer variables.
    • f943b2bf - vkd3d-shader/hlsl: Run constant passes in a separate function.
    • d85eecea - vkd3d-shader/hlsl: Run more constant passes on static expressions eval.
    • 85a92e74 - tests: Test complex array size expression.
    • 05cbfdf1 - vkd3d-shader/hlsl: Also lower matrix swizzles and index loads in const passes.

    Compare with previous version

  • Francisco Casas added 27 commits

    added 27 commits

    • 05cbfdf1...766913f9 - 21 commits from branch wine:master
    • 1fa08545 - tests: Test default values for uniform variables.
    • 95b1bf22 - tests: Test default values for constant buffer variables.
    • 13bfd1ad - vkd3d-shader/hlsl: Run constant passes in a separate function.
    • 0ac3f772 - vkd3d-shader/hlsl: Run more constant passes on static expressions eval.
    • 155e32a5 - tests: Test complex array size expression.
    • bdc0413e - vkd3d-shader/hlsl: Also lower matrix swizzles and index loads in const passes.

    Compare with previous version

  • Rebased on top of current master.

  • Francisco Casas mentioned in merge request !851 (merged)

    mentioned in merge request !851 (merged)

  • added 6 commits

    • c84ede70 - tests: Test default values for uniform variables.
    • d10d006c - tests: Test default values for constant buffer variables.
    • eb18c6ae - vkd3d-shader/hlsl: Run constant passes in a separate function.
    • 31738cce - vkd3d-shader/hlsl: Run more constant passes on static expressions eval.
    • f9ac97c3 - tests: Test complex array size expression.
    • 9faf2843 - vkd3d-shader/hlsl: Also lower matrix swizzles and index loads in const passes.

    Compare with previous version

  • Needs to be rebased again I'm afraid.

  • Francisco Casas added 64 commits

    added 64 commits

    • 9faf2843...4b3a948e - 58 commits from branch wine:master
    • 239dd21c - tests: Test default values for uniform variables.
    • a7e44b12 - tests: Test default values for constant buffer variables.
    • 1d4bd507 - vkd3d-shader/hlsl: Run constant passes in a separate function.
    • f0361e71 - vkd3d-shader/hlsl: Run more constant passes on static expressions eval.
    • 71a8bd43 - tests: Test complex array size expression.
    • 871b4bec - vkd3d-shader/hlsl: Also lower matrix swizzles and index loads in const passes.

    Compare with previous version

  • Francisco Casas added 36 commits

    added 36 commits

    • 871b4bec...98f73ca2 - 30 commits from branch wine:master
    • d9ca9d69 - tests: Test default values for uniform variables.
    • 201fe55f - tests: Test default values for constant buffer variables.
    • 96f4f646 - vkd3d-shader/hlsl: Run constant passes in a separate function.
    • eb1bea91 - vkd3d-shader/hlsl: Run more constant passes on static expressions eval.
    • a8b80515 - tests: Test complex array size expression.
    • dd8c13fd - vkd3d-shader/hlsl: Also lower matrix swizzles and index loads in const passes.

    Compare with previous version

  • Francisco Casas mentioned in merge request !787 (merged)

    mentioned in merge request !787 (merged)

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Henri Verbeet approved this merge request

    approved this merge request

  • added 17 commits

    • dd8c13fd...9e57039f - 11 commits from branch wine:master
    • 499b44a1 - tests: Test default values for uniform variables.
    • 58a6db55 - tests: Test default values for constant buffer variables.
    • 4f60c716 - vkd3d-shader/hlsl: Run constant passes in a separate function.
    • 0d252f89 - vkd3d-shader/hlsl: Run more constant passes on static expressions eval.
    • dff2f746 - tests: Test complex array size expression.
    • 061dc390 - vkd3d-shader/hlsl: Also lower matrix swizzles and index loads in const passes.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading