Update LongVector Execution tests to now use XML #7393

alsepkow · 2025-04-28T23:38:12Z

This change is mainly focused on swapping to use the XML and statically defined values. Opting to stage this for review in smaller chunk while I iterate on adding the additional test cases.

Update the LongVector exec tests to use the ShaderOpArithTable.xml TAEF table file for test entry points. This aligns with existing HLK tests.
Some light code cleanup.
Hard coded value sets in LongVectorTestData.h. Value sets give us a simple way to generate larger arrays from a smaller set of statically defined values. At a later point we can add logic to produce value sets at build time in this same header by consuming from a YAML file used in the offload test suite.

cleanup.

github-actions · 2025-04-28T23:38:58Z

✅ With the latest revision this PR passed the C/C++ code formatter.

damyanp · 2025-04-29T02:00:00Z

I'm guessing that the submodule changes aren't meant to be a part of this change?

alsepkow · 2025-04-29T02:01:49Z

I'm guessing that the submodule changes aren't meant to be a part of this change?

They are not. Accidentally grabbed those when I added the clang-format. Will fix.

tools/clang/unittests/HLSLExec/LongVectors.h

tools/clang/unittests/HLSLExec/LongVectorTestData.h

tools/clang/unittests/HLSLExec/ExecutionTest.cpp

alsepkow · 2025-04-30T02:56:13Z

tools/clang/unittests/HLSLExec/LongVectors.h

-  LongVectorOpType_Clamp,
-  LongVectorOpType_Initialize,
-  LongVectorOpType_UnInitialized
+enum LongVectorBinaryOpType {


Just want to note here that for subsequent PR's that add more tests cases I'm going to split these enums up by category instead of unary vs binary.
HLSLOperators, trigonometric, math, etc. And then I'll also have some child classes that inherit from LongVectorTestConfig for these various categories to help split out the logic by category and avoid large switch statements.

bob80905 · 2025-05-14T22:25:43Z

include/dxc/Test/HlslTestUtils.h

+  if (Src== Ref) {
+    return true;
+  }
+  if (std::isnan(Src)) {


What happens if Src is not nan and Ref is nan?

Good question :). I coped the pattern from other functions here but I'm not sure what would happen. I'll debug that case to see and report back.

bob80905 · 2025-05-14T22:28:57Z

include/dxc/Test/HlslTestUtils.h

-  if (fsrc == fref)
+  // Compare as floats to handle when we have '0 == -0' which should be true
+  // but won't be true if we compare as uint16_t.
+  if (ConvertFloat16ToFloat32(fsrc) == ConvertFloat16ToFloat32(fref))


Just curious, is there a ConvertUInt16ToFloat32 variant? Should that be created, or is the implementation of this conversion function identical to what would be done to convert uint16s?

That helper doesn't exist in this code. There is no float16 type in c++ (well, there actually is in c++23, but we don't use that yet). So float16 was implemented in a DX header in the Windows SDK.

So, this function name is a little confusing IMHO.

A little more context......
The two Convert* functions defined in HlslTestUtils reference their respective conversion functions in that DX SDK header. 'XMConvertHalfToFloat' and 'XMConvertFloatToHalf'. DirectX::PackedVector::HALF is aliased as a uint16_t in that header. And that uint16_t is used to store the bits of a 16-bit float. The DX header provides the functions to convert those bits into a floating-point value via the conversion functions, but because C++ had no float16 we use a float but the values are effectively clamped to the range of a 16 bit float.

The take-away is that we store and pass around float16 values as uint16_t's, but when it comes time to do comparisons we use the helper functions to interpret that uint16_t bit pattern as a float value.

Here's a link to the mentioned header if you're curious: DirectXPackedVector

bob80905 · 2025-05-14T22:30:48Z

tools/clang/unittests/HLSLExec/ExecutionTest.cpp

+    {L"DataType", TableParameter::STRING, true},
+    {L"OpTypeEnum", TableParameter::STRING, true},
+    {L"InputValueSetName1", TableParameter::STRING, false},
+    {L"InputArgsName", TableParameter::STRING, false},


Surprised BinaryOpParameters doesn't need something like InputArgsName.

It will eventually. None of the binary intrinsics added up to this point have required input arguments. Although, there might be a better name to use here. The 'InputArgNames' are used for additional arguments like the min/max values in clamp. Distinct from the unary, binary, or ternary operands.

bob80905 · 2025-05-14T22:34:43Z

tools/clang/unittests/HLSLExec/ExecutionTest.cpp

+    LongVectorOpTestDispatchByVectorSize<double>(OpType, Handler);
+  else
+    VERIFY_FAIL(
+        String().Format(L"DataType: %s is not recognized.", DataType.c_str()));


would this need to use %ls for wide strings or something of the sort?

tools/clang/unittests/HLSLExec/ExecutionTest.cpp

bob80905 · 2025-05-14T22:40:08Z

tools/clang/unittests/HLSLExec/ExecutionTest.cpp

+        }
+
+        LOG_ERROR_FMT_THROW(
+            L"RunShaderOpTest CallBack. Unexpected Resource Name: %S", Name);


Is %ls needed?

Name is an explicit LPCSTR (long pointer to a char). And we're formatting that into a wide character string. So, in this case %S is what we want.

This gets confusing because there are functions in Windows that can compile down to either a narrow (char*) or wide (wchar*) depending on the environment (ANSI vs Unicode). The format specifier characters can have different meanings based on which variant they end up in. This is helpful for writing portable code where the type of the string (wide or narrow) and the type of the string expected by the function could change based on the environment. When the types could change you want to use %S and %s

This is why windows API functions have things like 'CreateFile', 'CreateFileA', and 'CreateFileW'.

%ls will interpret the argument as a wide character string in both printf and wprintf.
%s is a char* in printf, but a wchar_t* in wprintf
%S is a wchar_t* in printf, but a char* in wprintf

Strings are 'fun' :).

bob80905 · 2025-05-14T22:46:12Z

tools/clang/unittests/HLSLExec/LongVectors.h

-  // on the seed value and are deterministic for any given seed.
-  std::mt19937 Generator;
+inline bool DoValuesMatch(HLSLBool_t A, HLSLBool_t B, [[maybe_unused]] float Tolerance, [[maybe_unused]] LongVector::ValidationType ValidationType) { 
+  return A == B;


Why doesn't this use the Tolerance and ValidationType args? Is it possible to just remove those 2 args?

This is an artifact of the templated functions. Theres a compiler warning enabled as a failure for unused function parameters. If I modify the calling location to only include these arguments based on the type it doesn't really solve much. It's called by DoArraysMatch a few lines below.

If you have a cleaner suggestion I'm open!

Not necessarily better, but this is another way I think to silence the warning:

DoValuesMatch(HLSLBool_t A, HLSLBool_t B, float, LongVector::ValidationType) {

bob80905 · 2025-05-14T22:46:51Z

tools/clang/unittests/HLSLExec/LongVectors.h

+template <typename T, std::size_t N>
+void LogLongVector(const std::array<T, N> &Values, const std::wstring &Name) {
+  WEX::Logging::Log::Comment(
+      WEX::Common::String().Format(L"LongVector Name: %s", Name.c_str()));


bob80905 · 2025-05-14T22:47:10Z

tools/clang/unittests/HLSLExec/LongVectors.h

+
+template <typename T> void LogScalar(const T &Value, const std::wstring &Name) {
+  WEX::Logging::Log::Comment(
+      WEX::Common::String().Format(L"Scalar Name: %s", Name.c_str()));


Update long vec execution tests to use xml and some other general

3fc0093

cleanup.

github-project-automation bot added this to HLSL Roadmap Apr 28, 2025

github-project-automation bot moved this to New in HLSL Roadmap Apr 28, 2025

Forgot clang format

c4d3de5

revert submodule changes

a8ae0da

damyanp reviewed Apr 29, 2025

View reviewed changes

alsepkow added 2 commits April 29, 2025 11:42

Code review

da4efc5

Clang-format

18135d6

alsepkow commented Apr 29, 2025

View reviewed changes

tools/clang/unittests/HLSLExec/LongVectors.h Outdated Show resolved Hide resolved

alsepkow commented Apr 29, 2025

View reviewed changes

tools/clang/unittests/HLSLExec/LongVectorTestData.h Outdated Show resolved Hide resolved

const args

80d4ce1

alsepkow commented Apr 30, 2025

View reviewed changes

tools/clang/unittests/HLSLExec/ExecutionTest.cpp Outdated Show resolved Hide resolved

alsepkow commented Apr 30, 2025

View reviewed changes

alsepkow added 9 commits April 30, 2025 23:00

WIP on trig. Accuracy issue and need some cleanup

482068a

Swapping

3849d62

Remove doubles, not supported for trig functions

70b675b

Wip, swap to look at coop vec stuff

51f3b2a

Trig tests passing

6d83f83

Bug fixes and remove mod float64 as it is not supported

7806a3c

A few bug fixes. Comparing 0 and -0

643f39c

Clang-format

26737e9

LLVM ifdef include guards

9ac2af4

bob80905 reviewed May 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update LongVector Execution tests to now use XML #7393

Update LongVector Execution tests to now use XML #7393

alsepkow commented Apr 28, 2025 •

edited by pow2clk

Loading

github-actions bot commented Apr 28, 2025 •

edited

Loading

damyanp commented Apr 29, 2025

alsepkow commented Apr 29, 2025

alsepkow Apr 30, 2025

bob80905 May 14, 2025

alsepkow May 14, 2025

bob80905 May 14, 2025

alsepkow May 14, 2025

bob80905 May 14, 2025

alsepkow May 14, 2025 •

edited

Loading

bob80905 May 14, 2025

bob80905 May 14, 2025

alsepkow May 15, 2025

bob80905 May 14, 2025

alsepkow May 15, 2025

damyanp May 15, 2025

bob80905 May 14, 2025

bob80905 May 14, 2025

Update LongVector Execution tests to now use XML #7393

Are you sure you want to change the base?

Update LongVector Execution tests to now use XML #7393

Conversation

alsepkow commented Apr 28, 2025 • edited by pow2clk Loading

github-actions bot commented Apr 28, 2025 • edited Loading

damyanp commented Apr 29, 2025

alsepkow commented Apr 29, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alsepkow May 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alsepkow commented Apr 28, 2025 •

edited by pow2clk

Loading

github-actions bot commented Apr 28, 2025 •

edited

Loading

alsepkow May 14, 2025 •

edited

Loading