From de079af20d0dd35115a3b630eba6c527aa7f5323 Mon Sep 17 00:00:00 2001 From: Harald van Dijk Date: Wed, 11 Oct 2023 11:17:05 +0100 Subject: [PATCH] [compiler] Flag uses of half types. When targeting a device that does not support half types, if we nonetheless received input that used half types, we would rely on LLVM's software implementation using library functions from libgcc/compiler-rt which we do not provide, and receive confusing linker errors as a result. For double types, we already flagged any use of such type as an error. This change extends that check to also flag half types, so that we issue an easier-to-understand error message. --- modules/compiler/source/base/CMakeLists.txt | 4 +- ...s.h => check_for_unsupported_types_pass.h} | 50 ++++++++++----- .../source/base_module_pass_machinery.cpp | 2 +- .../base/source/base_module_pass_registry.def | 2 +- ...p => check_for_unsupported_types_pass.cpp} | 64 +++++++++++-------- .../compiler/source/base/source/module.cpp | 4 +- .../lit/passes/check-for-doubles-optnone.ll | 5 +- .../test/lit/passes/check-for-doubles.ll | 5 +- .../lit/passes/check-for-halfs-optnone.ll | 26 ++++++++ .../test/lit/passes/check-for-halfs.ll | 25 ++++++++ 10 files changed, 132 insertions(+), 55 deletions(-) rename modules/compiler/source/base/include/base/{check_for_doubles_pass.h => check_for_unsupported_types_pass.h} (58%) rename modules/compiler/source/base/source/{check_for_doubles_pass.cpp => check_for_unsupported_types_pass.cpp} (56%) create mode 100644 modules/compiler/test/lit/passes/check-for-halfs-optnone.ll create mode 100644 modules/compiler/test/lit/passes/check-for-halfs.ll diff --git a/modules/compiler/source/base/CMakeLists.txt b/modules/compiler/source/base/CMakeLists.txt index 81e0a03d8..3b9eeed8a 100644 --- a/modules/compiler/source/base/CMakeLists.txt +++ b/modules/compiler/source/base/CMakeLists.txt @@ -33,8 +33,8 @@ set(COMPILER_BASE_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/include/base/module.h ${CMAKE_CURRENT_SOURCE_DIR}/include/base/bit_shift_fixup_pass.h ${CMAKE_CURRENT_SOURCE_DIR}/include/base/builtin_simplification_pass.h - ${CMAKE_CURRENT_SOURCE_DIR}/include/base/check_for_doubles_pass.h ${CMAKE_CURRENT_SOURCE_DIR}/include/base/check_for_ext_funcs_pass.h + ${CMAKE_CURRENT_SOURCE_DIR}/include/base/check_for_unsupported_types_pass.h ${CMAKE_CURRENT_SOURCE_DIR}/include/base/combine_fpext_fptrunc_pass.h ${CMAKE_CURRENT_SOURCE_DIR}/include/base/fast_math_pass.h ${CMAKE_CURRENT_SOURCE_DIR}/include/base/image_argument_substitution_pass.h @@ -50,8 +50,8 @@ set(COMPILER_BASE_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/source/module.cpp ${CMAKE_CURRENT_SOURCE_DIR}/source/bit_shift_fixup_pass.cpp ${CMAKE_CURRENT_SOURCE_DIR}/source/builtin_simplification_pass.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/source/check_for_doubles_pass.cpp ${CMAKE_CURRENT_SOURCE_DIR}/source/check_for_ext_funcs_pass.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/source/check_for_unsupported_types_pass.cpp ${CMAKE_CURRENT_SOURCE_DIR}/source/combine_fpext_fptrunc_pass.cpp ${CMAKE_CURRENT_SOURCE_DIR}/source/fast_math_pass.cpp ${CMAKE_CURRENT_SOURCE_DIR}/source/image_argument_substitution_pass.cpp diff --git a/modules/compiler/source/base/include/base/check_for_doubles_pass.h b/modules/compiler/source/base/include/base/check_for_unsupported_types_pass.h similarity index 58% rename from modules/compiler/source/base/include/base/check_for_doubles_pass.h rename to modules/compiler/source/base/include/base/check_for_unsupported_types_pass.h index 0d5352513..364061847 100644 --- a/modules/compiler/source/base/include/base/check_for_doubles_pass.h +++ b/modules/compiler/source/base/include/base/check_for_unsupported_types_pass.h @@ -16,10 +16,10 @@ /// @file /// -/// @brief Class CheckForDoublesPass +/// @brief Class CheckForUnsupportedTypesPass -#ifndef BASE_CHECK_FOR_DOUBLES_PASS_H_INCLUDED -#define BASE_CHECK_FOR_DOUBLES_PASS_H_INCLUDED +#ifndef BASE_CHECK_FOR_UNSUPPORTED_TYPES_PASS_H_INCLUDED +#define BASE_CHECK_FOR_UNSUPPORTED_TYPES_PASS_H_INCLUDED #include #include @@ -45,26 +45,42 @@ struct DiagnosticInfoDoubleNoDouble : public llvm::DiagnosticInfo { } }; +struct DiagnosticInfoHalfNoHalf : public llvm::DiagnosticInfo { + static int DK_HalfNoHalf; + + DiagnosticInfoHalfNoHalf() + : llvm::DiagnosticInfo(DK_HalfNoHalf, llvm::DS_Error) {} + + llvm::StringRef formatMessage() const; + + void print(llvm::DiagnosticPrinter &) const override; + + static bool classof(const llvm::DiagnosticInfo *DI) { + return DI->getKind() == DK_HalfNoHalf; + } +}; + /// @addtogroup cl_compiler /// @{ -/// @brief Pass to check for FP doubles. +/// @brief Pass to check for unsupported floating point types. /// -/// This pass is used to check for the presence of floating-point doubles. -/// Doubles are optional in OpenCL (cl_khr_fp64), and if we don't have them, we -/// need to check that we aren't using them. - -/// All basic blocks containing any instruction with 'double'-typed operands or -/// return type are raised as external functions are raised as -/// DiagnosticInfoDoubleNoDouble diagnostics with error-level severity. The -/// currently installed diagnostic handler is responsible for handling them. It -/// may abort, it may log an error and continue, or it may ignore them -/// completely; there are no requirements imposed by ComputeMux. +/// This pass is used to check for the presence of floating-point doubles and +/// halfs. Both are optional in OpenCL (cl_khr_fp64 and cl_khr_fp16), and if +/// we don't have them, we need to check that we aren't using them. + +/// All basic blocks containing any instruction with 'double'- or 'half'-typed +/// operands or return type are raised as external functions are raised as +/// DiagnosticInfoDoubleNoDouble or DiagnosticInfoHalfNoHalf diagnostics with +/// error-level severity. The currently installed diagnostic handler is +/// responsible for handling them. It may abort, it may log an error and +/// continue, or it may ignore them completely; there are no requirements +/// imposed by ComputeMux. /// /// Note that the compilation pipeline will continue after this pass unless the /// diagnostic stops it. -struct CheckForDoublesPass final - : public llvm::PassInfoMixin { +struct CheckForUnsupportedTypesPass final + : public llvm::PassInfoMixin { /// @brief The entry point to the pass. /// /// @param F The Function provided to the pass. @@ -82,4 +98,4 @@ struct CheckForDoublesPass final /// @} } // namespace compiler -#endif // BASE_CHECK_FOR_DOUBLES_PASS_H_INCLUDED +#endif // BASE_CHECK_FOR_UNSUPPORTED_TYPES_PASS_H_INCLUDED diff --git a/modules/compiler/source/base/source/base_module_pass_machinery.cpp b/modules/compiler/source/base/source/base_module_pass_machinery.cpp index 82f8e6184..b2c555393 100644 --- a/modules/compiler/source/base/source/base_module_pass_machinery.cpp +++ b/modules/compiler/source/base/source/base_module_pass_machinery.cpp @@ -17,8 +17,8 @@ #include #include #include -#include #include +#include #include #include #include diff --git a/modules/compiler/source/base/source/base_module_pass_registry.def b/modules/compiler/source/base/source/base_module_pass_registry.def index 59655f0d5..605aee590 100644 --- a/modules/compiler/source/base/source/base_module_pass_registry.def +++ b/modules/compiler/source/base/source/base_module_pass_registry.def @@ -170,7 +170,7 @@ FUNCTION_ANALYSIS("vectorize-metadata", compiler::utils::VectorizeMetadataAnalys FUNCTION_PASS("bit-shift-fixup", compiler::BitShiftFixupPass()) FUNCTION_PASS("ca-mem2reg", compiler::MemToRegPass()) -FUNCTION_PASS("check-doubles", compiler::CheckForDoublesPass()) +FUNCTION_PASS("check-unsupported-types", compiler::CheckForUnsupportedTypesPass()) FUNCTION_PASS("combine-fpext-fptrunc", compiler::CombineFPExtFPTruncPass()) FUNCTION_PASS("software-div", compiler::SoftwareDivisionPass()) FUNCTION_PASS("replace-addrspace-fns", compiler::utils::ReplaceAddressSpaceQualifierFunctionsPass()) diff --git a/modules/compiler/source/base/source/check_for_doubles_pass.cpp b/modules/compiler/source/base/source/check_for_unsupported_types_pass.cpp similarity index 56% rename from modules/compiler/source/base/source/check_for_doubles_pass.cpp rename to modules/compiler/source/base/source/check_for_unsupported_types_pass.cpp index c42b029b6..62c799799 100644 --- a/modules/compiler/source/base/source/check_for_doubles_pass.cpp +++ b/modules/compiler/source/base/source/check_for_unsupported_types_pass.cpp @@ -14,7 +14,7 @@ // // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#include +#include #include #include #include @@ -33,41 +33,53 @@ void compiler::DiagnosticInfoDoubleNoDouble::print(DiagnosticPrinter &P) const { P << formatMessage(); } -namespace { -// Tracks presence of double types in a BasicBlock. -inline bool findDoubleTypes(const BasicBlock &BB) { - // Each instruction in the BB - for (const auto &I : BB) { - // The instruction's return type - if (I.getType()->isDoubleTy()) { - return true; - } - // Else check each operand of the instruction - for (const auto &Op : I.operands()) { - if (Op->getType()->isDoubleTy()) { - return true; - } - } - } - return false; +int compiler::DiagnosticInfoHalfNoHalf::DK_HalfNoHalf = + getNextAvailablePluginDiagnosticKind(); + +StringRef compiler::DiagnosticInfoHalfNoHalf::formatMessage() const { + return "A half precision floating point number was generated, " + "but cl_khr_fp16 is not supported on this target."; +} + +void compiler::DiagnosticInfoHalfNoHalf::print(DiagnosticPrinter &P) const { + P << formatMessage(); } -} // namespace -PreservedAnalyses compiler::CheckForDoublesPass::run( +PreservedAnalyses compiler::CheckForUnsupportedTypesPass::run( Function &F, FunctionAnalysisManager &AM) { const auto &MAMProxy = AM.getResult(F); const auto *DI = MAMProxy.getCachedResult( *F.getParent()); - - // Check if doubles are supported, in which case there's nothing to do. - if (DI && DI->double_capabilities != 0) { + // Check if double and half are supported + bool CheckDouble = !DI || !DI->double_capabilities; + bool CheckHalf = !DI || !DI->half_capabilities; + // If we do not have to check for either type, exit right away. + if (!CheckDouble && !CheckHalf) { return PreservedAnalyses::all(); } - for (const auto &BB : F) { - if (findDoubleTypes(BB)) { + auto CheckType = [&](llvm::Type *T) { + if (T->isDoubleTy() && CheckDouble) { F.getContext().diagnose(DiagnosticInfoDoubleNoDouble()); - return PreservedAnalyses::all(); + CheckDouble = false; + } + if (T->isHalfTy() && CheckHalf) { + F.getContext().diagnose(DiagnosticInfoHalfNoHalf()); + CheckHalf = false; + } + }; + for (const auto &BB : F) { + for (const auto &I : BB) { + CheckType(I.getType()); + if (!CheckDouble && !CheckHalf) { + return PreservedAnalyses::all(); + } + for (const auto &Op : I.operands()) { + CheckType(Op->getType()); + if (!CheckDouble && !CheckHalf) { + return PreservedAnalyses::all(); + } + } } } return PreservedAnalyses::all(); diff --git a/modules/compiler/source/base/source/module.cpp b/modules/compiler/source/base/source/module.cpp index 54d7402a5..9a22aeb36 100644 --- a/modules/compiler/source/base/source/module.cpp +++ b/modules/compiler/source/base/source/module.cpp @@ -17,8 +17,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -1776,7 +1776,7 @@ Result BaseModule::finalize( { llvm::FunctionPassManager fpm; fpm.addPass(compiler::CombineFPExtFPTruncPass()); - fpm.addPass(compiler::CheckForDoublesPass()); + fpm.addPass(compiler::CheckForUnsupportedTypesPass()); pm.addPass(llvm::createModuleToFunctionPassAdaptor(std::move(fpm))); } diff --git a/modules/compiler/test/lit/passes/check-for-doubles-optnone.ll b/modules/compiler/test/lit/passes/check-for-doubles-optnone.ll index 006d3125d..7edf910d4 100644 --- a/modules/compiler/test/lit/passes/check-for-doubles-optnone.ll +++ b/modules/compiler/test/lit/passes/check-for-doubles-optnone.ll @@ -14,8 +14,8 @@ ; ; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -; RUN: muxc --passes "require,check-doubles" -S %s 2>&1 -; RUN: not muxc --passes check-doubles -S %s 2>&1 | FileCheck %s --check-prefix ERROR +; RUN: muxc --passes "require,check-unsupported-types" -S %s 2>&1 +; RUN: not muxc --passes check-unsupported-types -S %s 2>&1 | FileCheck %s --check-prefix ERROR ; ERROR: error: A double precision floating point number was generated, but cl_khr_fp64 is not supported on this target. @@ -23,5 +23,4 @@ define void @foo() optnone noinline { %a = fadd double 0.0, 1.0 ret void - } diff --git a/modules/compiler/test/lit/passes/check-for-doubles.ll b/modules/compiler/test/lit/passes/check-for-doubles.ll index 5b182a2f6..2122285f5 100644 --- a/modules/compiler/test/lit/passes/check-for-doubles.ll +++ b/modules/compiler/test/lit/passes/check-for-doubles.ll @@ -14,13 +14,12 @@ ; ; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -; RUN: muxc --passes "require,check-doubles" -S %s 2>&1 -; RUN: not muxc --passes check-doubles -S %s 2>&1 | FileCheck %s --check-prefix ERROR +; RUN: muxc --passes "require,check-unsupported-types" -S %s 2>&1 +; RUN: not muxc --passes check-unsupported-types -S %s 2>&1 | FileCheck %s --check-prefix ERROR ; ERROR: error: A double precision floating point number was generated, but cl_khr_fp64 is not supported on this target. define void @foo() { %a = fadd double 0.0, 1.0 ret void - } diff --git a/modules/compiler/test/lit/passes/check-for-halfs-optnone.ll b/modules/compiler/test/lit/passes/check-for-halfs-optnone.ll new file mode 100644 index 000000000..a795760b3 --- /dev/null +++ b/modules/compiler/test/lit/passes/check-for-halfs-optnone.ll @@ -0,0 +1,26 @@ +; Copyright (C) Codeplay Software Limited +; +; Licensed under the Apache License, Version 2.0 (the "License") with LLVM +; Exceptions; you may not use this file except in compliance with the License. +; You may obtain a copy of the License at +; +; https://github.com/codeplaysoftware/oneapi-construction-kit/blob/main/LICENSE.txt +; +; Unless required by applicable law or agreed to in writing, software +; distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +; WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +; License for the specific language governing permissions and limitations +; under the License. +; +; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +; RUN: muxc --passes "require,check-unsupported-types" -S %s 2>&1 +; RUN: not muxc --passes check-unsupported-types -S %s 2>&1 | FileCheck %s --check-prefix ERROR + +; ERROR: error: A half precision floating point number was generated, but cl_khr_fp16 is not supported on this target. + +; Check that optnone functions aren't skipped +define void @foo() optnone noinline { + %a = fadd half 0.0, 1.0 + ret void +} diff --git a/modules/compiler/test/lit/passes/check-for-halfs.ll b/modules/compiler/test/lit/passes/check-for-halfs.ll new file mode 100644 index 000000000..22595799e --- /dev/null +++ b/modules/compiler/test/lit/passes/check-for-halfs.ll @@ -0,0 +1,25 @@ +; Copyright (C) Codeplay Software Limited +; +; Licensed under the Apache License, Version 2.0 (the "License") with LLVM +; Exceptions; you may not use this file except in compliance with the License. +; You may obtain a copy of the License at +; +; https://github.com/codeplaysoftware/oneapi-construction-kit/blob/main/LICENSE.txt +; +; Unless required by applicable law or agreed to in writing, software +; distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +; WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +; License for the specific language governing permissions and limitations +; under the License. +; +; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +; RUN: muxc --passes "require,check-unsupported-types" -S %s 2>&1 +; RUN: not muxc --passes check-unsupported-types -S %s 2>&1 | FileCheck %s --check-prefix ERROR + +; ERROR: error: A half precision floating point number was generated, but cl_khr_fp16 is not supported on this target. + +define void @foo() { + %a = fadd half 0.0, 1.0 + ret void +}