Implement ULP Comparison for doubles (#203)

spall · web-flow · commit 198e13e3cf52 · 2025-05-27T08:28:20.000-07:00
* compare ulp for doubles

* clang format

* capitalize Fn + add L to tests

* forgot requires double

* respond to pr comments
diff --git a/lib/Support/Check.cpp b/lib/Support/Check.cpp
@@ -14,12 +14,35 @@
 #include "llvm/Support/raw_ostream.h"
 #include <cmath>
 
-static bool isDenorm(float F) { return std::fpclassify(F) == FP_SUBNORMAL; }
+// limited to float, double, and long double
+template <typename T> static bool isDenorm(T F) {
+  return std::fpclassify(F) == FP_SUBNORMAL;
+}
 
 static bool isFloat16NAN(uint16_t Val) {
   return (Val & 0x7c00) == 0x7c00 && (Val & 0x03ff) != 0;
 }
 
+static bool compareDoubleULP(const double &FSrc, const double &FRef,
+                             unsigned ULPTolerance,
+                             offloadtest::DenormMode DM) {
+  if (FSrc == FRef)
+    return true;
+  if (std::isnan(FSrc) || std::isnan(FRef))
+    return std::isnan(FRef) && std::isnan(FSrc);
+  if (DM == offloadtest::DenormMode::Any) {
+    // If denorm expected, output can be sign preserved zero. Otherwise output
+    // should pass the regular ulp testing.
+    if (isDenorm(FRef) && FSrc == 0 && std::signbit(FSrc) == std::signbit(FRef))
+      return true;
+  }
+  // For FTZ or Preserve mode, we should get the expected number within
+  // ULPTolerance for any operations.
+  const int64_t Diff = *((const uint64_t *)&FSrc) - *((const uint64_t *)&FRef);
+  const uint64_t AbsDiff = Diff < 0 ? -Diff : Diff;
+  return AbsDiff <= ULPTolerance;
+}
+
 static bool compareFloatULP(const float &FSrc, const float &FRef,
                             unsigned ULPTolerance, offloadtest::DenormMode DM) {
   if (FSrc == FRef)
@@ -61,40 +84,54 @@ static bool testBufferExact(offloadtest::Buffer *B1, offloadtest::Buffer *B2) {
   return true;
 }
 
+template <typename T>
+static bool testAll(std::function<bool(const T &, const T &)> ComparisonFn,
+                    llvm::ArrayRef<T> Arr1, llvm::ArrayRef<T> Arr2) {
+  if (Arr1.size() != Arr2.size())
+    return false;
+
+  for (size_t I = 0, E = Arr1.size(); I < E; ++I) {
+    if (!ComparisonFn(Arr1[I], Arr2[I]))
+      return false;
+  }
+  return true;
+}
+
 static bool testBufferFuzzy(offloadtest::Buffer *B1, offloadtest::Buffer *B2,
                             unsigned ULPT, offloadtest::DenormMode DM) {
+  assert(B1->Format == B2->Format && "Buffer types must be the same");
   switch (B1->Format) {
+  case offloadtest::DataFormat::Float64: {
+    const llvm::ArrayRef<double> Arr1(
+        reinterpret_cast<double *>(B1->Data.get()), B1->Size / sizeof(double));
+    const llvm::ArrayRef<double> Arr2(
+        reinterpret_cast<double *>(B2->Data.get()), B2->Size / sizeof(double));
+    auto Fn = [ULPT, DM](const double &FS, const double &FR) {
+      return compareDoubleULP(FS, FR, ULPT, DM);
+    };
+    return testAll<double>(Fn, Arr1, Arr2);
+  }
   case offloadtest::DataFormat::Float32: {
-    if (B1->Size != B2->Size)
-      return false;
     const llvm::ArrayRef<float> Arr1(reinterpret_cast<float *>(B1->Data.get()),
                                      B1->Size / sizeof(float));
-    assert(B2->Format == offloadtest::DataFormat::Float32 &&
-           "Buffer types must be the same");
     const llvm::ArrayRef<float> Arr2(reinterpret_cast<float *>(B2->Data.get()),
                                      B2->Size / sizeof(float));
-    for (unsigned I = 0, E = Arr1.size(); I < E; ++I) {
-      if (!compareFloatULP(Arr1[I], Arr2[I], ULPT, DM))
-        return false;
-    }
-    return true;
+    auto Fn = [ULPT, DM](const float &FS, const float &FR) {
+      return compareFloatULP(FS, FR, ULPT, DM);
+    };
+    return testAll<float>(Fn, Arr1, Arr2);
   }
   case offloadtest::DataFormat::Float16: {
-    if (B1->Size != B2->Size)
-      return false;
     const llvm::ArrayRef<uint16_t> Arr1(
         reinterpret_cast<uint16_t *>(B1->Data.get()),
         B1->Size / sizeof(uint16_t));
-    assert(B2->Format == offloadtest::DataFormat::Float16 &&
-           "Buffer types must be the same");
     const llvm::ArrayRef<uint16_t> Arr2(
         reinterpret_cast<uint16_t *>(B2->Data.get()),
         B2->Size / sizeof(uint16_t));
-    for (unsigned I = 0, E = Arr1.size(); I < E; ++I) {
-      if (!compareFloat16ULP(Arr1[I], Arr2[I], ULPT))
-        return false;
-    }
-    return true;
+    auto Fn = [ULPT](const uint16_t &FS, const uint16_t &FR) {
+      return compareFloat16ULP(FS, FR, ULPT);
+    };
+    return testAll<uint16_t>(Fn, Arr1, Arr2);
   }
   default:
     llvm_unreachable("Only float types are supported by the fuzzy test.");
diff --git a/lib/Support/Pipeline.cpp b/lib/Support/Pipeline.cpp
@@ -14,7 +14,8 @@
 using namespace offloadtest;
 
 static bool isFloatingPointFormat(DataFormat Format) {
-  return Format == DataFormat::Float16 || Format == DataFormat::Float32;
+  return Format == DataFormat::Float16 || Format == DataFormat::Float32 ||
+         Format == DataFormat::Float64;
 }
 
 namespace llvm {
@@ -49,6 +50,8 @@ void MappingTraits<offloadtest::Pipeline>::mapping(IO &I,
         if (!isFloatingPointFormat(R.ActualPtr->Format) ||
             !isFloatingPointFormat(R.ExpectedPtr->Format))
           I.setError(Twine("BufferFuzzy only accepts Float buffers"));
+        if (R.ActualPtr->Format != R.ExpectedPtr->Format)
+          I.setError(Twine("Buffers must have the same type"));
       }
     }
 
diff --git a/test/Tools/Offloader/BufferFuzzy-64bit.test b/test/Tools/Offloader/BufferFuzzy-64bit.test
@@ -0,0 +1,114 @@
+#--- source.hlsl
+
+RWStructuredBuffer<double> Out1 : register(u0);
+RWStructuredBuffer<double> Out2 : register(u1);
+RWStructuredBuffer<double> Out3 : register(u2);
+RWStructuredBuffer<double> Out4 : register(u3);
+
+[numthreads(1,1,1)]
+void main() {
+  Out1[0] = 0.0;
+  Out2[0] = 3.14159265L; // Need L or clang will resolve this to a float and test will fail
+  Out3[0] = asdouble(0x00000000, 0x7FF80000); // Should be NaN
+  Out4[0] = 5.4L;   // Need L or clang will resolve this to a float and test will fail
+}
+
+//--- pipeline.yaml
+
+---
+Shaders:
+  - Stage: Compute
+    Entry: main
+    DispatchSize: [1, 1, 1]
+Buffers:
+  - Name: Out1
+    Format: Float64
+    Stride: 8
+    ZeroInitSize: 8
+  - Name: Expected1
+    Format: Float64
+    Stride: 8
+    Data: [ 0x0.fffffffffffffp-1022 ] # isDenorm will return true for this value: 2.22507385850720104E-308
+  - Name: Out2
+    Format: Float64
+    Stride: 8
+    ZeroInitSize: 8
+  - Name: Expected2
+    Format: Float64
+    Stride: 8
+    Data: [ 3.14159265 ]
+  - Name: Out3
+    Format: Float64
+    Stride: 8
+    ZeroInitSize: 8
+  - Name: Expected3
+    Format: Float64
+    Stride: 8
+    Data: [ NaN ]
+  - Name: Out4
+    Format: Float64
+    Stride: 8
+    ZeroInitSize: 8
+  - Name: Expected4
+    Format: Float64
+    Stride: 8
+    Data: [ 5.399999999999999 ] # Should be 2 ulp away
+Results:
+  - Result: Test1 # Testing Expected is Denorm and Out is zero, and both have same sign bit
+    Rule: BufferFuzzy
+    ULPT: 1
+    DenormMode: Any
+    Actual: Out1
+    Expected: Expected1
+  - Result: Test2 # Test two values are exactly the same
+    Rule: BufferFuzzy
+    ULPT: 0 # ulp shouldn't matter for this test
+    Actual: Out2
+    Expected: Expected2
+  - Result: Test3 # Test both are NaN
+    Rule: BufferFuzzy
+    ULPT: 0 # ulp shouldn't matter for this test
+    Actual: Out3
+    Expected: Expected3
+  - Result: Test4 # Actual and Expected are not the same but they are within the ULPT
+    Rule: BufferFuzzy
+    ULPT: 2
+    Actual: Out4
+    Expected: Expected4
+DescriptorSets:
+  - Resources:
+    - Name: Out1
+      Kind: RWStructuredBuffer
+      DirectXBinding:
+        Register: 0
+        Space: 0
+      VulkanBinding:
+        Binding: 0
+    - Name: Out2
+      Kind: RWStructuredBuffer
+      DirectXBinding:
+        Register: 1
+        Space: 0
+      VulkanBinding:
+        Binding: 1
+    - Name: Out3
+      Kind: RWStructuredBuffer
+      DirectXBinding:
+        Register: 2
+        Space: 0
+      VulkanBinding:
+        Binding: 2
+    - Name: Out4
+      Kind: RWStructuredBuffer
+      DirectXBinding:
+        Register: 3
+        Space: 0
+      VulkanBinding:
+        Binding: 3
+...
+#--- end
+
+# REQUIRES: Double
+# RUN: split-file %s %t
+# RUN: %dxc_target -T cs_6_5 -Fo %t.o %t/source.hlsl
+# RUN: %offloader %t/pipeline.yaml %t.o
diff --git a/test/Tools/Offloader/BufferFuzzy-error-64bit.test b/test/Tools/Offloader/BufferFuzzy-error-64bit.test

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,8 @@`
`14`	`14`	`using namespace offloadtest;`
`15`	`15`
`16`	`16`	`static bool isFloatingPointFormat(DataFormat Format) {`
`17`		`- return Format == DataFormat::Float16 \|\| Format == DataFormat::Float32;`
	`17`	`+ return Format == DataFormat::Float16 \|\| Format == DataFormat::Float32 \|\|`
	`18`	`+ Format == DataFormat::Float64;`
`18`	`19`	`}`
`19`	`20`
`20`	`21`	`namespace llvm {`
`@@ -49,6 +50,8 @@ void MappingTraits<offloadtest::Pipeline>::mapping(IO &I,`
`49`	`50`	`if (!isFloatingPointFormat(R.ActualPtr->Format) \|\|`
`50`	`51`	`!isFloatingPointFormat(R.ExpectedPtr->Format))`
`51`	`52`	`I.setError(Twine("BufferFuzzy only accepts Float buffers"));`
	`53`	`+ if (R.ActualPtr->Format != R.ExpectedPtr->Format)`
	`54`	`+ I.setError(Twine("Buffers must have the same type"));`
`52`	`55`	`}`
`53`	`56`	`}`
`54`	`57`