From a7ef0f089585d43f1c4f0e2302d693249f453b7b Mon Sep 17 00:00:00 2001 From: Natalie Chouinard Date: Mon, 8 Apr 2024 16:09:51 +0000 Subject: [PATCH 1/2] [SPIR-V] Map {min,max} to N{Min,Max} SPIR-V intsruction selection was mapping the LLVM min/max instrinsics to FMin and FMax respectively for GL/Vulkan environments, which does not match the intrinsic's documented treatment of NaN operands. This patch switches the mapping to the correctly matched NMin and NMax operations. Fixes #87072 --- llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp | 4 ++-- llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll | 6 +++--- llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp index 49749b5634530..45a70da7f8690 100644 --- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp @@ -432,10 +432,10 @@ bool SPIRVInstructionSelector::spvSelect(Register ResVReg, case TargetOpcode::G_FMINNUM: case TargetOpcode::G_FMINIMUM: - return selectExtInst(ResVReg, ResType, I, CL::fmin, GL::FMin); + return selectExtInst(ResVReg, ResType, I, CL::fmin, GL::NMin); case TargetOpcode::G_FMAXNUM: case TargetOpcode::G_FMAXIMUM: - return selectExtInst(ResVReg, ResType, I, CL::fmax, GL::FMax); + return selectExtInst(ResVReg, ResType, I, CL::fmax, GL::NMax); case TargetOpcode::G_FCOPYSIGN: return selectExtInst(ResVReg, ResType, I, CL::copysign); diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll index 48e916581f9ff..8cad0e20e5156 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll @@ -5,21 +5,21 @@ define noundef half @test_fmax_half(half noundef %a, half noundef %b) { entry: -; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] FMax %[[#]] %[[#]] +; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] NMax %[[#]] %[[#]] %0 = call half @llvm.maxnum.f16(half %a, half %b) ret half %0 } define noundef float @test_fmax_float(float noundef %a, float noundef %b) { entry: -; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] FMax %[[#]] %[[#]] +; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] NMax %[[#]] %[[#]] %0 = call float @llvm.maxnum.f32(float %a, float %b) ret float %0 } define noundef double @test_fmax_double(double noundef %a, double noundef %b) { entry: -; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] FMax %[[#]] %[[#]] +; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] NMax %[[#]] %[[#]] %0 = call double @llvm.maxnum.f64(double %a, double %b) ret double %0 } diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll index 5bfd69c972a3f..fa2fceb5f6368 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll @@ -7,21 +7,21 @@ define noundef half @test_fmax_half(half noundef %a, half noundef %b) { entry: -; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] FMin %[[#]] %[[#]] +; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] NMin %[[#]] %[[#]] %0 = call half @llvm.minnum.f16(half %a, half %b) ret half %0 } define noundef float @test_fmax_float(float noundef %a, float noundef %b) { entry: -; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] FMin %[[#]] %[[#]] +; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] NMin %[[#]] %[[#]] %0 = call float @llvm.minnum.f32(float %a, float %b) ret float %0 } define noundef double @test_fmax_double(double noundef %a, double noundef %b) { entry: -; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] FMin %[[#]] %[[#]] +; CHECK: %[[#]] = OpExtInst %[[#]] %[[#]] NMin %[[#]] %[[#]] %0 = call double @llvm.minnum.f64(double %a, double %b) ret double %0 } From abc803f8791990ed51b498efa204bf6ecff0d5a0 Mon Sep 17 00:00:00 2001 From: Natalie Chouinard Date: Mon, 8 Apr 2024 18:38:54 +0000 Subject: [PATCH 2/2] Remove TODOs --- llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll | 1 - llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll | 1 - 2 files changed, 2 deletions(-) diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll index 8cad0e20e5156..159d4ac19c8cc 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmax.ll @@ -1,6 +1,5 @@ ; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s ; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %} -; TODO: This need to be NMax: See https://github.com/llvm/llvm-project/issues/87072 ; CHECK: OpExtInstImport "GLSL.std.450" define noundef half @test_fmax_half(half noundef %a, half noundef %b) { diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll index fa2fceb5f6368..15946b5038eec 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/fmin.ll @@ -1,6 +1,5 @@ ; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s ; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %} -; TODO: This need to be NMin: See https://github.com/llvm/llvm-project/issues/87072 ; CHECK: OpExtInstImport "GLSL.std.450" ; CHECK: OpMemoryModel Logical GLSL450