[spirv] Translate HLSL min/max to NMin/NMax (#4517)

sudonatalie · web-flow · commit 2a0833afb425 · 2022-06-17T16:17:54.000-04:00
The specification for the HLSL min and max intrinsic functions states that if one of the values is NaN, the other will be given as the result, which is correctly represented by the GLSL.std.450 instructions NMin and NMax, respectively. By the semantics of the previously used FMin and FMax instructions, this would be undefined behavior. Fixes #3221
diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst
@@ -2357,8 +2357,8 @@ HLSL Intrinsic Function   GLSL Extended Instruction
 ``log10``               ``Log2`` (scaled by ``1/log2(10)``)
 ``log2``                ``Log2``
 ``mad``                 ``Fma``
-``max``                 ``SMax``/``UMax``/``FMax``
-``min``                 ``SMin``/``UMin``/``FMin``
+``max``                 ``SMax``/``UMax``/``NMax``
+``min``                 ``SMin``/``UMin``/``NMin``
 ``modf``                ``ModfStruct``
 ``normalize``           ``Normalize``
 ``pow``                 ``Pow``
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -8136,9 +8136,9 @@ SpirvEmitter::processIntrinsicCallExpr(const CallExpr *callExpr) {
     INTRINSIC_OP_CASE(lerp, FMix, true);
     INTRINSIC_OP_CASE(log, Log, true);
     INTRINSIC_OP_CASE(log2, Log2, true);
-    INTRINSIC_OP_CASE_SINT_UINT_FLOAT(max, SMax, UMax, FMax, true);
+    INTRINSIC_OP_CASE_SINT_UINT_FLOAT(max, SMax, UMax, NMax, true);
     INTRINSIC_OP_CASE(umax, UMax, true);
-    INTRINSIC_OP_CASE_SINT_UINT_FLOAT(min, SMin, UMin, FMin, true);
+    INTRINSIC_OP_CASE_SINT_UINT_FLOAT(min, SMin, UMin, NMin, true);
     INTRINSIC_OP_CASE(umin, UMin, true);
     INTRINSIC_OP_CASE(normalize, Normalize, false);
     INTRINSIC_OP_CASE(pow, Pow, true);
diff --git a/tools/clang/test/CodeGenSPIRV/intrinsics.max.hlsl b/tools/clang/test/CodeGenSPIRV/intrinsics.max.hlsl
@@ -1,5 +1,9 @@
 // RUN: %dxc -T vs_6_0 -E main
 
+// According to HLSL reference on denormals for 'max', if one of the
+// values is NaN, the other will be given as the result. If both values
+// are NaN, the result will be NaN.
+
 // CHECK:      [[glsl:%\d+]] = OpExtInstImport "GLSL.std.450"
 
 void main() {
@@ -19,38 +23,38 @@ void main() {
   uint3 j1,j2;
   uresult3 = max(j1,j2);
 
-// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] FMax {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] NMax {{%\d+}} {{%\d+}}
   float a1,a2;
   result = max(a1,a2);
 
-// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] FMax {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] NMax {{%\d+}} {{%\d+}}
   float1 b1,b2;
   result = max(b1,b2);
 
-// CHECK: {{%\d+}} = OpExtInst %v3float [[glsl]] FMax {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %v3float [[glsl]] NMax {{%\d+}} {{%\d+}}
   float3 c1,c2;
   result3 = max(c1,c2);
 
-// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] FMax {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] NMax {{%\d+}} {{%\d+}}
   float1x1 d1,d2;
   result = max(d1,d2);
 
-// CHECK: {{%\d+}} = OpExtInst %v2float [[glsl]] FMax {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %v2float [[glsl]] NMax {{%\d+}} {{%\d+}}
   float1x2 e1,e2;
   result2 = max(e1,e2);
 
-// CHECK: {{%\d+}} = OpExtInst %v4float [[glsl]] FMax {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %v4float [[glsl]] NMax {{%\d+}} {{%\d+}}
   float4x1 f1,f2;
   result4 = max(f1,f2);
 
 // CHECK:      [[g1:%\d+]] = OpLoad %mat2v3float %g1
 // CHECK-NEXT: [[g2:%\d+]] = OpLoad %mat2v3float %g2
 // CHECK-NEXT: [[g1_row0:%\d+]] = OpCompositeExtract %v3float [[g1]] 0
 // CHECK-NEXT: [[g2_row0:%\d+]] = OpCompositeExtract %v3float [[g2]] 0
-// CHECK-NEXT: [[result_row0:%\d+]] = OpExtInst %v3float [[glsl]] FMax [[g1_row0]] [[g2_row0]]
+// CHECK-NEXT: [[result_row0:%\d+]] = OpExtInst %v3float [[glsl]] NMax [[g1_row0]] [[g2_row0]]
 // CHECK-NEXT: [[g1_row1:%\d+]] = OpCompositeExtract %v3float [[g1]] 1
 // CHECK-NEXT: [[g2_row1:%\d+]] = OpCompositeExtract %v3float [[g2]] 1
-// CHECK-NEXT: [[result_row1:%\d+]] = OpExtInst %v3float [[glsl]] FMax [[g1_row1]] [[g2_row1]]
+// CHECK-NEXT: [[result_row1:%\d+]] = OpExtInst %v3float [[glsl]] NMax [[g1_row1]] [[g2_row1]]
 // CHECK-NEXT: {{%\d+}} = OpCompositeConstruct %mat2v3float [[result_row0]] [[result_row1]]
   float2x3 g1,g2;
   result2x3 = max(g1,g2);
diff --git a/tools/clang/test/CodeGenSPIRV/intrinsics.min.hlsl b/tools/clang/test/CodeGenSPIRV/intrinsics.min.hlsl
@@ -1,5 +1,9 @@
 // RUN: %dxc -T vs_6_0 -E main
 
+// According to HLSL reference on denormals for 'min', if one of the
+// values is NaN, the other will be given as the result. If both values
+// are NaN, the result will be NaN.
+
 // CHECK:      [[glsl:%\d+]] = OpExtInstImport "GLSL.std.450"
 
 void main() {
@@ -19,38 +23,38 @@ void main() {
   uint3 j1,j2;
   uresult3 = min(j1,j2);
 
-// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] FMin {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] NMin {{%\d+}} {{%\d+}}
   float a1,a2;
   result = min(a1,a2);
 
-// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] FMin {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] NMin {{%\d+}} {{%\d+}}
   float1 b1,b2;
   result = min(b1,b2);
 
-// CHECK: {{%\d+}} = OpExtInst %v3float [[glsl]] FMin {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %v3float [[glsl]] NMin {{%\d+}} {{%\d+}}
   float3 c1,c2;
   result3 = min(c1,c2);
 
-// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] FMin {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %float [[glsl]] NMin {{%\d+}} {{%\d+}}
   float1x1 d1,d2;
   result = min(d1,d2);
 
-// CHECK: {{%\d+}} = OpExtInst %v2float [[glsl]] FMin {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %v2float [[glsl]] NMin {{%\d+}} {{%\d+}}
   float1x2 e1,e2;
   result2 = min(e1,e2);
 
-// CHECK: {{%\d+}} = OpExtInst %v4float [[glsl]] FMin {{%\d+}} {{%\d+}}
+// CHECK: {{%\d+}} = OpExtInst %v4float [[glsl]] NMin {{%\d+}} {{%\d+}}
   float4x1 f1,f2;
   result4 = min(f1,f2);
 
 // CHECK:      [[g1:%\d+]] = OpLoad %mat2v3float %g1
 // CHECK-NEXT: [[g2:%\d+]] = OpLoad %mat2v3float %g2
 // CHECK-NEXT: [[g1_row0:%\d+]] = OpCompositeExtract %v3float [[g1]] 0
 // CHECK-NEXT: [[g2_row0:%\d+]] = OpCompositeExtract %v3float [[g2]] 0
-// CHECK-NEXT: [[result_row0:%\d+]] = OpExtInst %v3float [[glsl]] FMin [[g1_row0]] [[g2_row0]]
+// CHECK-NEXT: [[result_row0:%\d+]] = OpExtInst %v3float [[glsl]] NMin [[g1_row0]] [[g2_row0]]
 // CHECK-NEXT: [[g1_row1:%\d+]] = OpCompositeExtract %v3float [[g1]] 1
 // CHECK-NEXT: [[g2_row1:%\d+]] = OpCompositeExtract %v3float [[g2]] 1
-// CHECK-NEXT: [[result_row1:%\d+]] = OpExtInst %v3float [[glsl]] FMin [[g1_row1]] [[g2_row1]]
+// CHECK-NEXT: [[result_row1:%\d+]] = OpExtInst %v3float [[glsl]] NMin [[g1_row1]] [[g2_row1]]
 // CHECK-NEXT: {{%\d+}} = OpCompositeConstruct %mat2v3float [[result_row0]] [[result_row1]]
   float2x3 g1,g2;
   result2x3 = min(g1,g2);
diff --git a/tools/clang/test/CodeGenSPIRV/shader.debug.line.intrinsic.hlsl b/tools/clang/test/CodeGenSPIRV/shader.debug.line.intrinsic.hlsl
@@ -182,5 +182,5 @@ void main() {
 // CHECK-NEXT: OpExtInst %float {{%\d+}} Cos
       cos(v4f.x));
 // CHECK:      DebugLine [[src]] %uint_180 %uint_183 %uint_3 %uint_17
-// CHECK-NEXT: OpExtInst %float {{%\d+}} FMax
+// CHECK-NEXT: OpExtInst %float {{%\d+}} NMax
 }
diff --git a/tools/clang/test/CodeGenSPIRV/spirv.debug.opline.intrinsic.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.debug.opline.intrinsic.hlsl
@@ -182,5 +182,5 @@ void main() {
 // CHECK-NEXT: OpExtInst %float {{%\d+}} Cos
       cos(v4f.x));
 // CHECK:      OpLine [[file]] 180 3
-// CHECK-NEXT: OpExtInst %float {{%\d+}} FMax
+// CHECK-NEXT: OpExtInst %float {{%\d+}} NMax
 }

Original file line number	Diff line number	Diff line change
`@@ -182,5 +182,5 @@ void main() {`
`182`	`182`	`// CHECK-NEXT: OpExtInst %float {{%\d+}} Cos`
`183`	`183`	`cos(v4f.x));`
`184`	`184`	`// CHECK: DebugLine [[src]] %uint_180 %uint_183 %uint_3 %uint_17`
`185`		`-// CHECK-NEXT: OpExtInst %float {{%\d+}} FMax`
	`185`	`+// CHECK-NEXT: OpExtInst %float {{%\d+}} NMax`
`186`	`186`	`}`