Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[d3d9] workaround for EDG frontend based compilers #3800

Closed

Conversation

r-a-sattarov
Copy link
Contributor

@r-a-sattarov r-a-sattarov commented Jan 17, 2024

EDG frontend based compilers can't find suitable constructor to convert from "const int *"

lcc: "../src/d3d9/d3d9_state.h", line 306: error #415: no suitable constructor
          exists to convert from "const int *" to "dxvk::Vector4Base<float>"
              set->fConsts[StartRegister + i] = replaceNaN(pConstantData + (i * 4));
                                                           ^
          detected during:
            instantiation of
                      "HRESULT dxvk::UpdateStateConstants<ProgramType,ConstantType,T,StateType>(StateType *, UINT, const T *, UINT, bool) [with ProgramType=dxvk::DxsoProgramTypes::VertexShader, ConstantType=dxvk::D3D9ConstantType::Int, T=int, StateType=dxvk::D3D9CapturableState]"
                      at line 374 of "../src/d3d9/d3d9_stateblock.h"
            instantiation of
                      "HRESULT dxvk::D3D9StateBlock::SetShaderConstants<ProgramType,ConstantType,T>(UINT, const T *, UINT) [with ProgramType=dxvk::DxsoProgramTypes::VertexShader, ConstantType=dxvk::D3D9ConstantType::Int, T=int]"
                      at line 6871 of "../src/d3d9/d3d9_device.cpp"
            instantiation of
                      "HRESULT dxvk::D3D9DeviceEx::SetShaderConstants<ProgramType,ConstantType,T>(UINT, const T *, UINT) [with ProgramType=dxvk::DxsoProgramTypes::VertexShader, ConstantType=dxvk::D3D9ConstantType::Int, T=int]"
                      at line 3143 of "../src/d3d9/d3d9_device.cpp"

1 error detected in the compilation of "../src/d3d9/d3d9_device.cpp".

Added workaround for this case.

@r-a-sattarov
Copy link
Contributor Author

r-a-sattarov commented Jan 17, 2024

@daveedvdv
Hello Daveed. Sorry for mentioning you, but could you help us with bug in EDG frontend?

@Triang3l
Copy link

Triang3l commented Jan 18, 2024

Although relying on the order of passes within the compiler definitely sounds a bit like depending on undefined behavior (though I'm not sure, maybe the expectations from if constexpr are actually well-defined and it's a compiler bug), I think this specific workaround would introduce a caller/callee responsibility reversal.

According to this example, it should be possible to explicitly specialize a template based on enum values. So, it's possible to implement "callbacks" for the three types either by specializing a function (however, it appears that functions may be specialized only fully — and that would restrict the type T of pConstantData to something exact, but here that may even be desirable for additional validation to catch errors like this earlier, though a much bigger issue here is D3D9ShaderConstantsVSSoftware versus D3D9ShaderConstantsPS), or by partially specializing a class template (somewhat like that std::vector<bool> special case in the standard library) with one method.

template <
  D3D9ConstantType ConstantType,
  typename         T,
  typename         StateType>
class StateConstantUpdater {};

template <
  typename T,
  typename StateType>
class StateConstantUpdater<D3D9ConstantType::Float, T, StateType> {
public:
  static HRESULT UpdateStateConstants(
          StateType*           pState,
          UINT                 StartRegister,
    const T*                   pConstantData,
          UINT                 Count,
          bool                 FloatEmu) {
    if (!FloatEmu) {
      size_t size = Count * sizeof(Vector4);

      std::memcpy(set->fConsts[StartRegister].data, pConstantData, size);
    }
    else {
      for (UINT i = 0; i < Count; i++)
        set->fConsts[StartRegister + i] = replaceNaN(pConstantData + (i * 4));
    }

    return D3D_OK;
  }
};

template <
  typename T,
  typename StateType>
class StateConstantUpdater<D3D9ConstantType::Int, T, StateType> {
public:
  static HRESULT UpdateStateConstants(
          StateType*           pState,
          UINT                 StartRegister,
    const T*                   pConstantData,
          UINT                 Count,
          bool                 FloatEmu) {
    size_t size = Count * sizeof(Vector4i);

    std::memcpy(set->iConsts[StartRegister].data, pConstantData, size);

    return D3D_OK;
  }
};

template <
  typename T,
  typename StateType>
class StateConstantUpdater<D3D9ConstantType::Bool, T, StateType> {
public:
  static HRESULT UpdateStateConstants(
          StateType*           pState,
          UINT                 StartRegister,
    const T*                   pConstantData,
          UINT                 Count,
          bool                 FloatEmu) {
    for (uint32_t i = 0; i < Count; i++) {
      const uint32_t constantIdx = StartRegister + i;
      const uint32_t arrayIdx    = constantIdx / 32;
      const uint32_t bitIdx      = constantIdx % 32;

      const uint32_t bit = 1u << bitIdx;

      set->bConsts[arrayIdx] &= ~bit;
      if (pConstantData[i])
        set->bConsts[arrayIdx] |= bit;
    }

    return D3D_OK;
  }
};

Though I haven't checked, but maybe for nested templates (like a template class in a template function, or even a template function in a template function — in which case maybe it can be fully specialized) it should even be possible to reuse parameters:

  template <
    DxsoProgramType  ProgramType,
    D3D9ConstantType ConstantType,
    typename         T,
    typename         StateType>
  HRESULT UpdateStateConstants(
          StateType*           pState,
          UINT                 StartRegister,
    const T*                   pConstantData,
          UINT                 Count,
          bool                 FloatEmu) {
    template <D3D9ConstantType ConstantType>
    class StateConstantUpdater {};

    template <>
    class StateConstantUpdater<D3D9ConstantType::Float> {
    public:
      static HRESULT UpdateStateConstants(
              StateType*           pState,
              UINT                 StartRegister,
        const T*                   pConstantData,
              UINT                 Count,
              bool                 FloatEmu) {
        if (!FloatEmu) {
          size_t size = Count * sizeof(Vector4);

          std::memcpy(set->fConsts[StartRegister].data, pConstantData, size);
        }
        else {
          for (UINT i = 0; i < Count; i++)
            set->fConsts[StartRegister + i] = replaceNaN(pConstantData + (i * 4));
        }

        return D3D_OK;
      }
    };

    template <>
    class StateConstantUpdater<D3D9ConstantType::Int> {
    public:
      static HRESULT UpdateStateConstants(
              StateType*           pState,
              UINT                 StartRegister,
        const T*                   pConstantData,
              UINT                 Count,
              bool                 FloatEmu) {
        size_t size = Count * sizeof(Vector4i);

        std::memcpy(set->iConsts[StartRegister].data, pConstantData, size);

        return D3D_OK;
      }
    };

    template <>
    class StateConstantUpdater<D3D9ConstantType::Bool> {
    public:
      static HRESULT UpdateStateConstants(
              StateType*           pState,
              UINT                 StartRegister,
        const T*                   pConstantData,
              UINT                 Count,
              bool                 FloatEmu) {
        for (uint32_t i = 0; i < Count; i++) {
          const uint32_t constantIdx = StartRegister + i;
          const uint32_t arrayIdx    = constantIdx / 32;
          const uint32_t bitIdx      = constantIdx % 32;

          const uint32_t bit = 1u << bitIdx;

          set->bConsts[arrayIdx] &= ~bit;
          if (pConstantData[i])
            set->bConsts[arrayIdx] |= bit;
        }

        return D3D_OK;
      }
    };

    return ProgramType == DxsoProgramTypes::VertexShader
      ? StateConstantUpdater<ConstantType>::UpdateStateConstants(pState->vsConsts,
                                                                 StartRegister,
                                                                 pConstantData,
                                                                 Count,
                                                                 FloatEmu)
      : StateConstantUpdater<ConstantType>::UpdateStateConstants(pState->psConsts,
                                                                 StartRegister,
                                                                 pConstantData,
                                                                 Count,
                                                                 FloatEmu);
  }

(or the same with a = delete base function, and full per-constant-type specializations)

Another option would be to replace the D3D9ConstantType enumeration with three separate empty classes, and using function overloading for the selection everywhere (similarly to the idea behind things like std::piecewise_construct_t), but that'd require much wider code changes.

@r-a-sattarov r-a-sattarov force-pushed the edg-frontend-workaround branch 2 times, most recently from 10b7e95 to 5e75ec8 Compare January 23, 2024 19:25
@r-a-sattarov
Copy link
Contributor Author

Although relying on the order of passes within the compiler definitely sounds a bit like depending on undefined behavior (though I'm not sure, maybe the expectations from if constexpr are actually well-defined and it's a compiler bug), I think this specific workaround would introduce a caller/callee responsibility reversal.

I redid my patch.

Copy link

@Toxblh Toxblh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/d3d9/d3d9_state.h Outdated Show resolved Hide resolved
@K0bin
Copy link
Collaborator

K0bin commented Sep 17, 2024

Sorry, but I gotta ask: Why are you even using that compiler? Why not just use GCC, MSVC or Clang?

@doitsujin doitsujin closed this Sep 26, 2024
@r-a-sattarov
Copy link
Contributor Author

Sorry, but I gotta ask: Why are you even using that compiler? Why not just use GCC, MSVC or Clang?

Hallo Robin! Sorry for long reply.

On my hardware platform (Elbrus-2000) only one compiler is available so far and it uses the EDG frontend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants