Skip to content

Add comparison rule, tolerance if applicable, and hex64 view to test failure text output #291

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.

Sign up for GitHub

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
+201 −40
Open

Add comparison rule, tolerance if applicable, and hex64 view to test failure text output #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
796dd2e
add comparison rule, tolerance if applicable, and hex64 view to test …
bob80905 Jul 17, 2025
d2f03d1
make sure hex representation is correct
bob80905 Jul 18, 2025
1dccd9c
address build errors
bob80905 Jul 18, 2025
21c5b1e
fix typo
bob80905 Jul 18, 2025
78467e7
print as strings, use standard hex float format for C++
bob80905 Jul 21, 2025
d9ee686
try applying clangformat
bob80905 Jul 21, 2025
db2f5ea
clang format
bob80905 Jul 21, 2025
cde6fee
add hex data text after the yaml
bob80905 Jul 22, 2025
f8fd606
try clang-format
bob80905 Jul 22, 2025
7a7634a
address build errors
bob80905 Jul 22, 2025
edb7c30
use std::hex for exact tests
bob80905 Jul 22, 2025
bb0b384
address Alex
bob80905 Jul 23, 2025
0d167eb
address Alex + Finn
bob80905 Jul 25, 2025
4bc8e29
fix missing output props bug
bob80905 Jul 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 93 additions & 3 deletions lib/Support/Check.cpp
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Do we want to add a test case of the correct error message output?

We could use FileCheck to ensure the hex values or whatnot are being formatted as expected

Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
//===----------------------------------------------------------------------===//

#include "Support/Check.h"
#include "Support/Pipeline.h"
#include "llvm/ADT/APFloat.h"
#include "llvm/ADT/APInt.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/raw_ostream.h"
#include <cmath>
#include <sstream>

constexpr uint16_t Float16BitSign = 0x8000;
constexpr uint16_t Float16BitExp = 0x7c00;
Expand Down Expand Up @@ -267,30 +269,118 @@ static bool testBufferFloatULP(offloadtest::Buffer *B1, offloadtest::Buffer *B2,
return false;
}

template <typename T>
static std::string bitPatternAsHex64(const T &Val,
offloadtest::Rule ComparisonRule) {
static_assert(sizeof(T) <= sizeof(uint64_t), "Type too large for Hex64");

std::ostringstream Oss;
if (ComparisonRule == offloadtest::Rule::BufferExact)
Copy link

Choose a reason for hiding this comment

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

Could we use llvm::raw_svector_ostream to avoid the include of ? Or is not able to handle the conversion of std::hex?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The std library is the only thing aware of std::hex, unfortunately none of the other llvm ostreams are compatible with std::hex / hexfloat.

Oss << std::hex << Val;
else
Oss << std::hexfloat << Val;
return Oss.str();
}

template <typename T>
std::string formatBuffer(offloadtest::Buffer *B, offloadtest::Rule rule) {
llvm::MutableArrayRef<T> arr(reinterpret_cast<T *>(B->Data.get()),
B->Size / sizeof(T));
if (arr.empty())
return "";

std::string result = "[ " + bitPatternAsHex64(arr[0], rule);
for (size_t i = 1; i < arr.size(); ++i)
result += ", " + bitPatternAsHex64(arr[i], rule);
result += " ]";
return result;
}

static const std::string getBufferStr(offloadtest::Buffer *B,
Copy link

Choose a reason for hiding this comment

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

Up to you, but I think it would be cleaner to use templates for this instead of a macro.
I asked co-pilot to re-write using templates.

Also includes a simplification of the logic of the for loop formatting the output string.

Edit: After writing this comment I see the same pattern used elsewhere in Pipeline.cpp. The templated version still seems cleaner IMO. And I do see the same pattern with ENUM_CASE in pipeline.h as well, but all of those cases have very simple logic in the macro. I'll still leave the decision up to you.

Something like this:

template <typename T>
std::string formatBuffer(offloadtest::Buffer* B, offloadtest::Rule rule) {
  llvm::MutableArrayRef<T> arr(reinterpret_cast<T*>(B->Data.get()), B->Size / sizeof(T));
  if (arr.empty()) return "";

  std::string result = "[ " + bitPatternAsHex64(arr[0], rule);
  for (size_t i = 1; i < arr.size(); ++i)
    result += ", " + bitPatternAsHex64(arr[i], rule);
  result += " ]";
  return result;
}

static const std::string getBufferStr(offloadtest::Buffer* B, offloadtest::Rule rule) {
  using DF = offloadtest::DataFormat;
  switch (B->Format) {
    case DF::Hex8:    return formatBuffer<llvm::yaml::Hex8>(B, rule);
    case DF::Hex16:   return formatBuffer<llvm::yaml::Hex16>(B, rule);
    case DF::Hex32:   return formatBuffer<llvm::yaml::Hex32>(B, rule);
    case DF::Hex64:   return formatBuffer<llvm::yaml::Hex64>(B, rule);
    case DF::UInt16:  return formatBuffer<uint16_t>(B, rule);
    case DF::UInt32:  return formatBuffer<uint32_t>(B, rule);
    case DF::UInt64:  return formatBuffer<uint64_t>(B, rule);
    case DF::Int16:   return formatBuffer<int16_t>(B, rule);
    case DF::Int32:   return formatBuffer<int32_t>(B, rule);
    case DF::Int64:   return formatBuffer<int64_t>(B, rule);
    case DF::Float16: return formatBuffer<llvm::yaml::Hex16>(B, rule); // assuming no native float16
    case DF::Float32: return formatBuffer<float>(B, rule);
    case DF::Float64: return formatBuffer<double>(B, rule);
    case DF::Bool:    return formatBuffer<uint32_t>(B, rule); // Because sizeof(bool) is 1 but HLSL represents a bool using 4 bytes.
    default:          return "UHO SCOOBY";
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I would +1 on the templated version fwiw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed Pipeline.cpp and Check.cpp, I think I should leave pipeline.h as is.

offloadtest::Rule rule) {
using DF = offloadtest::DataFormat;
switch (B->Format) {
case DF::Hex8:
return formatBuffer<llvm::yaml::Hex8>(B, rule);
case DF::Hex16:
return formatBuffer<llvm::yaml::Hex16>(B, rule);
case DF::Hex32:
return formatBuffer<llvm::yaml::Hex32>(B, rule);
case DF::Hex64:
return formatBuffer<llvm::yaml::Hex64>(B, rule);
case DF::UInt16:
return formatBuffer<uint16_t>(B, rule);
case DF::UInt32:
return formatBuffer<uint32_t>(B, rule);
case DF::UInt64:
return formatBuffer<uint64_t>(B, rule);
case DF::Int16:
return formatBuffer<int16_t>(B, rule);
case DF::Int32:
return formatBuffer<int32_t>(B, rule);
case DF::Int64:
return formatBuffer<int64_t>(B, rule);
case DF::Float16:
return formatBuffer<llvm::yaml::Hex16>(B,
rule); // assuming no native float16
case DF::Float32:
return formatBuffer<float>(B, rule);
case DF::Float64:
return formatBuffer<double>(B, rule);
case DF::Bool:
return formatBuffer<uint32_t>(B,
rule); // Because sizeof(bool) is 1 but HLSL
// represents a bool using 4 bytes.
}
}

llvm::Error verifyResult(offloadtest::Result R) {
llvm::SmallString<256> Str;
llvm::raw_svector_ostream OS(Str);
OS << "Test failed: " << R.Name << "\n";

switch (R.ComparisonRule) {
case offloadtest::Rule::BufferExact: {
if (testBufferExact(R.ActualPtr, R.ExpectedPtr))
return llvm::Error::success();
OS << "Comparison Rule: BufferExact\n";
break;
}
case offloadtest::Rule::BufferFloatULP: {
if (testBufferFloatULP(R.ActualPtr, R.ExpectedPtr, R.ULPT, R.DM))
return llvm::Error::success();
OS << "Comparison Rule: BufferFloatULP\nULP: " << R.ULPT << "\n";
break;
}
case offloadtest::Rule::BufferFloatEpsilon: {
if (testBufferFloatEpsilon(R.ActualPtr, R.ExpectedPtr, R.Epsilon, R.DM))
return llvm::Error::success();

std::ostringstream Oss;
Oss << std::defaultfloat << R.Epsilon;
OS << "Comparison Rule: BufferFloatEpsilon\nEpsilon: " << Oss.str() << "\n";
break;
}
}
llvm::SmallString<256> Str;
llvm::raw_svector_ostream OS(Str);
OS << "Test failed: " << R.Name << "\nExpected:\n";

OS << "Expected:\n";
llvm::yaml::Output YAMLOS(OS);
YAMLOS << *R.ExpectedPtr;
OS << "Got:\n";
YAMLOS << *R.ActualPtr;

// Now print exact hex64 representations of each element of the
// actual and expected buffers.

const std::string ExpectedBufferStr =
getBufferStr(R.ExpectedPtr, R.ComparisonRule);
const std::string ActualBufferStr =
getBufferStr(R.ActualPtr, R.ComparisonRule);

OS << "Full Hex 64bit representation of Expected Buffer Values:\n"
<< ExpectedBufferStr << "\n";
OS << "Full Hex 64bit representation of Actual Buffer Values:\n"
<< ActualBufferStr << "\n";

return llvm::createStringError(Str.c_str());
}
92 changes: 55 additions & 37 deletions lib/Support/Pipeline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,31 @@ void MappingTraits<offloadtest::DescriptorSet>::mapping(
I.mapRequired("Resources", D.Resources);
}

template <typename Type> static void setData(IO &I, offloadtest::Buffer &B) {
if (I.outputting()) {
llvm::MutableArrayRef<Type> Arr(reinterpret_cast<Type *>(B.Data.get()),
B.Size / sizeof(Type));
I.mapRequired("Data", Arr);
} else {
int64_t ZeroInitSize;
I.mapOptional("ZeroInitSize", ZeroInitSize, 0);
if (ZeroInitSize > 0) {
B.Size = ZeroInitSize;
B.Data.reset(new char[B.Size]);
memset(B.Data.get(), 0, B.Size);
I.mapOptional("OutputProps", B.OutputProps);
return;
}
llvm::SmallVector<Type, 64> Arr;
I.mapRequired("Data", Arr);
B.Size = Arr.size() * sizeof(Type);
B.Data.reset(new char[B.Size]);
memcpy(B.Data.get(), Arr.data(), B.Size);
}

I.mapOptional("OutputProps", B.OutputProps);
}

void MappingTraits<offloadtest::Buffer>::mapping(IO &I,
offloadtest::Buffer &B) {
I.mapRequired("Name", B.Name);
Expand All @@ -102,44 +127,37 @@ void MappingTraits<offloadtest::Buffer>::mapping(IO &I,
I.mapOptional("Counter", B.Counter, 0);
if (!I.outputting() && B.Stride != 0 && B.Channels != 1)
I.setError("Cannot set a structure stride and more than one channel.");
using DF = offloadtest::DataFormat;
switch (B.Format) {
#define DATA_CASE(Enum, Type) \
case DataFormat::Enum: { \
if (I.outputting()) { \
llvm::MutableArrayRef<Type> Arr(reinterpret_cast<Type *>(B.Data.get()), \
B.Size / sizeof(Type)); \
I.mapRequired("Data", Arr); \
} else { \
int64_t ZeroInitSize; \
I.mapOptional("ZeroInitSize", ZeroInitSize, 0); \
if (ZeroInitSize > 0) { \
B.Size = ZeroInitSize; \
B.Data.reset(new char[B.Size]); \
memset(B.Data.get(), 0, B.Size); \
break; \
} \
llvm::SmallVector<Type, 64> Arr; \
I.mapRequired("Data", Arr); \
B.Size = Arr.size() * sizeof(Type); \
B.Data.reset(new char[B.Size]); \
memcpy(B.Data.get(), Arr.data(), B.Size); \
} \
break; \
}
DATA_CASE(Hex8, llvm::yaml::Hex8)
DATA_CASE(Hex16, llvm::yaml::Hex16)
DATA_CASE(Hex32, llvm::yaml::Hex32)
DATA_CASE(Hex64, llvm::yaml::Hex64)
DATA_CASE(UInt16, uint16_t)
DATA_CASE(UInt32, uint32_t)
DATA_CASE(UInt64, uint64_t)
DATA_CASE(Int16, int16_t)
DATA_CASE(Int32, int32_t)
DATA_CASE(Int64, int64_t)
DATA_CASE(Float16, llvm::yaml::Hex16)
DATA_CASE(Float32, float)
DATA_CASE(Float64, double)
DATA_CASE(Bool, uint32_t) // Because sizeof(bool) is 1 but HLSL represents a bool using 4 bytes.
case DF::Hex8:
return setData<llvm::yaml::Hex8>(I, B);
case DF::Hex16:
return setData<llvm::yaml::Hex16>(I, B);
case DF::Hex32:
return setData<llvm::yaml::Hex32>(I, B);
case DF::Hex64:
return setData<llvm::yaml::Hex64>(I, B);
case DF::UInt16:
return setData<uint16_t>(I, B);
case DF::UInt32:
return setData<uint32_t>(I, B);
case DF::UInt64:
return setData<uint64_t>(I, B);
case DF::Int16:
return setData<int16_t>(I, B);
case DF::Int32:
return setData<int32_t>(I, B);
case DF::Int64:
return setData<int64_t>(I, B);
case DF::Float16:
return setData<llvm::yaml::Hex16>(I, B); // assuming no native float16
case DF::Float32:
return setData<float>(I, B);
case DF::Float64:
return setData<double>(I, B);
case DF::Bool:
return setData<uint32_t>(I, B); // Because sizeof(bool) is 1 but HLSL
// represents a bool using 4 bytes.
}

I.mapOptional("OutputProps", B.OutputProps);
Expand Down
5 changes: 5 additions & 0 deletions test/Tools/Offloader/BufferExact-error.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ DescriptorSets:
# RUN: not %offloader %t/pipeline.yaml %t.o 2>&1 | FileCheck %s

# CHECK: Test failed: Test1
# CHECK: Comparison Rule: BufferExact
# CHECK: Expected:
# CHECK: ---
# CHECK: Name: Expected1
Expand All @@ -69,3 +70,7 @@ DescriptorSets:
# CHECK: Height: 0
# CHECK: Width: 0
# CHECK: Depth: 0
# CHECK: Full Hex 64bit representation of Expected Buffer Values:
# CHECK: [ 1, 2, 3, 4 ]
# CHECK: Full Hex 64bit representation of Actual Buffer Values:
# CHECK: [ 14, 1e, 28, 32 ]
48 changes: 48 additions & 0 deletions test/Tools/Offloader/BufferFloat-error-64bit.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ DescriptorSets:
# RUN: not %offloader %t/pipeline.yaml %t.o 2>&1 | FileCheck %s

# CHECK: Test failed: Test1
# CHECK: Comparison Rule: BufferFloatULP
# CHECK: ULP: 1
# CHECK: Expected:
# CHECK: ---
# CHECK: Name: Expected1
Expand All @@ -131,8 +133,18 @@ DescriptorSets:
# CHECK: Height: 0
# CHECK: Width: 0
# CHECK: Depth: 0
# CHECK: ...
# CHECK-NEXT: Full Hex 64bit representation of Expected Buffer Values:
# CHECK-NEXT: [ 0x1.8000000000000p+0, 0x1.4000000000000p+1 ]
# CHECK-NEXT: Full Hex 64bit representation of Actual Buffer Values:
# CHECK-NEXT: [ 0x1.44cccc
# The rest is #ccccccdp+4, 0x1.4000000000000p+2 ], but some implementations
# the remaining hex64 data. So, we resume checking from p+4
# CHECK: p+4, 0x1.4000000000000p+2 ]

# CHECK: Test failed: Test2
# CHECK: Comparison Rule: BufferFloatULP
# CHECK: ULP: 1
# CHECK: Expected:
# CHECK: ---
# CHECK: Name: Expected2
Expand All @@ -154,8 +166,15 @@ DescriptorSets:
# CHECK: Height: 0
# CHECK: Width: 0
# CHECK: Depth: 0
# CHECK: ...
# CHECK-NEXT: Full Hex 64bit representation of Expected Buffer Values:
# CHECK-NEXT: [ 0x0.fffffffffffffp-1022 ]
# CHECK-NEXT: Full Hex 64bit representation of Actual Buffer Values:
# CHECK-NEXT: [ 0x0.0000000000000p+0 ]

# CHECK: Test failed: Test3
# CHECK: Comparison Rule: BufferFloatULP
# CHECK: ULP: 0
# CHECK: Expected:
# CHECK: ---
# CHECK: Name: Expected3
Expand All @@ -177,8 +196,15 @@ DescriptorSets:
# CHECK: Height: 0
# CHECK: Width: 0
# CHECK: Depth: 0
# CHECK: ...
# CHECK-NEXT: Full Hex 64bit representation of Expected Buffer Values:
# CHECK-NEXT: [ 0x0.0000000000000p+0 ]
# CHECK-NEXT: Full Hex 64bit representation of Actual Buffer Values:
# CHECK-NEXT: [ nan ]

# CHECK: Test failed: Test4
# CHECK: Comparison Rule: BufferFloatEpsilon
# CHECK: Epsilon: 0.1
# CHECK: Expected:
# CHECK: ---
# CHECK: Name: Expected1
Expand All @@ -200,8 +226,18 @@ DescriptorSets:
# CHECK: Height: 0
# CHECK: Width: 0
# CHECK: Depth: 0
# CHECK: ...
# CHECK-NEXT: Full Hex 64bit representation of Expected Buffer Values:
# CHECK-NEXT: [ 0x1.8000000000000p+0, 0x1.4000000000000p+1 ]
# CHECK-NEXT: Full Hex 64bit representation of Actual Buffer Values:
# CHECK-NEXT: [ 0x1.44cccc
# The rest is #ccccccdp+4, 0x1.4000000000000p+2 ], but some implementations
# the remaining hex64 data. So, we resume checking from p+4
# CHECK: p+4, 0x1.4000000000000p+2 ]

# CHECK: Test failed: Test5
# CHECK: Comparison Rule: BufferFloatEpsilon
# CHECK: Epsilon: 0
# CHECK: Expected:
# CHECK: ---
# CHECK: Name: Expected2
Expand All @@ -223,8 +259,15 @@ DescriptorSets:
# CHECK: Height: 0
# CHECK: Width: 0
# CHECK: Depth: 0
# CHECK: ...
# CHECK-NEXT: Full Hex 64bit representation of Expected Buffer Values:
# CHECK-NEXT: [ 0x0.fffffffffffffp-1022 ]
# CHECK-NEXT: Full Hex 64bit representation of Actual Buffer Values:
# CHECK-NEXT: [ 0x0.0000000000000p+0 ]

# CHECK: Test failed: Test6
# CHECK: Comparison Rule: BufferFloatEpsilon
# CHECK: Epsilon: 0
# CHECK: Expected:
# CHECK: ---
# CHECK: Name: Expected3
Expand All @@ -246,3 +289,8 @@ DescriptorSets:
# CHECK: Height: 0
# CHECK: Width: 0
# CHECK: Depth: 0
# CHECK: ...
# CHECK-NEXT: Full Hex 64bit representation of Expected Buffer Values:
# CHECK-NEXT: [ 0x0.0000000000000p+0 ]
# CHECK-NEXT: Full Hex 64bit representation of Actual Buffer Values:
# CHECK-NEXT: [ nan ]
Loading