Accept intrinsic namings into -spirv-allow-unknown-intrinsics (#912)

Previously, enabling the option would let all intrinsics through
the translator. From now on, the option can be configured so
that only specific intrinsic calls with the given prefix are
allowed, while those not matching the prefix will result in an
error as if no option was given.

The allowed intrinsic prefixes are fed into the option as a
comma-separated list, akin to `--spirv-ext`. The list can also
be left unspecified, which, as before, will accept all intrinsic
calls into SPIR-V.

One of the relevant use cases involves llvm.genx.* intrinsics,
which stem from SYCL ESIMD code, but can also mix with common LLVM
intrinsics that stem from regular SYCL code. It would be desirable
to allow ESIMD-specific intrinsics, but still error out on other
unknown ones, To better highlight this case, all ESIMD-specific
tests have been switched to passing an explicit prefix to the
CLI option.

Author: Artem Gindinson

include/LLVMSPIRVOpts.h

@@ -39,11 +39,19 @@
 #ifndef SPIRV_LLVMSPIRVOPTS_H
 #define SPIRV_LLVMSPIRVOPTS_H
 
+#include <llvm/ADT/Optional.h>
+#include <llvm/ADT/SmallVector.h>
+#include <llvm/ADT/StringRef.h>
+
 #include <cassert>
 #include <cstdint>
 #include <map>
 #include <unordered_map>
 
+namespace llvm {
+class IntrinsicInst;
+} // namespace llvm
+
 namespace SPIRV {
 
 enum class VersionNumber : uint32_t {
@@ -77,6 +85,7 @@ enum class DebugInfoEIS : uint32_t { SPIRV_Debug, OpenCL_DebugInfo_100 };
 class TranslatorOpts {
 public:
   using ExtensionsStatusMap = std::map<ExtensionID, bool>;
+  using ArgList = llvm::SmallVector<llvm::StringRef, 4>;
 
   TranslatorOpts() = default;
 
@@ -139,14 +148,9 @@ class TranslatorOpts {
 
   FPContractMode getFPContractMode() const { return FPCMode; }
 
-  bool isSPIRVAllowUnknownIntrinsicsEnabled() const noexcept {
-    return SPIRVAllowUnknownIntrinsics;
-  }
-
-  void
-  setSPIRVAllowUnknownIntrinsicsEnabled(bool AllowUnknownIntrinsics) noexcept {
-    SPIRVAllowUnknownIntrinsics = AllowUnknownIntrinsics;
-  }
+  bool isUnknownIntrinsicAllowed(llvm::IntrinsicInst *II) const noexcept;
+  bool isSPIRVAllowUnknownIntrinsicsEnabled() const noexcept;
+  void setSPIRVAllowUnknownIntrinsics(ArgList IntrinsicPrefixList) noexcept;
 
   bool allowExtraDIExpressions() const noexcept {
     return AllowExtraDIExpressions;
@@ -193,7 +197,7 @@ class TranslatorOpts {
 
   // Unknown LLVM intrinsics will be translated as external function calls in
   // SPIR-V
-  bool SPIRVAllowUnknownIntrinsics = false;
+  llvm::Optional<ArgList> SPIRVAllowUnknownIntrinsics{};
 
   // Enable support for extra DIExpression opcodes not listed in the SPIR-V
   // DebugInfo specification.

lib/SPIRV/CMakeLists.txt

@@ -1,4 +1,5 @@
 add_llvm_library(LLVMSPIRVLib
+  LLVMSPIRVOpts.cpp
   LLVMToSPIRVDbgTran.cpp
   Mangler/FunctionDescriptor.cpp
   Mangler/Mangler.cpp

lib/SPIRV/LLVMSPIRVOpts.cpp

@@ -0,0 +1,71 @@
+//===- LLVMSPIRVOpts.cpp - Defines LLVM/SPIR-V options ----------*- C++ -*-===//
+//
+//                     The LLVM/SPIR-V Translator
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+// Copyright (c) 2021 Intel Corporation. All rights reserved.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the "Software"),
+// to deal with the Software without restriction, including without limitation
+// the rights to use, copy, modify, merge, publish, distribute, sublicense,
+// and/or sell copies of the Software, and to permit persons to whom the
+// Software is furnished to do so, subject to the following conditions:
+//
+// Redistributions of source code must retain the above copyright notice,
+// this list of conditions and the following disclaimers.
+// Redistributions in binary form must reproduce the above copyright notice,
+// this list of conditions and the following disclaimers in the documentation
+// and/or other materials provided with the distribution.
+// Neither the names of Advanced Micro Devices, Inc., nor the names of its
+// contributors may be used to endorse or promote products derived from this
+// Software without specific prior written permission.
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// CONTRIBUTORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS WITH
+// THE SOFTWARE.
+//
+//===----------------------------------------------------------------------===//
+/// \file
+///
+/// This file provides definitions for LLVM/SPIR-V Translator's CLI
+/// functionality.
+///
+//===----------------------------------------------------------------------===//
+
+#include "LLVMSPIRVOpts.h"
+
+#include <llvm/ADT/Optional.h>
+#include <llvm/ADT/SmallVector.h>
+#include <llvm/ADT/StringRef.h>
+#include <llvm/IR/IntrinsicInst.h>
+
+using namespace llvm;
+using namespace SPIRV;
+
+bool TranslatorOpts::isUnknownIntrinsicAllowed(IntrinsicInst *II) const
+    noexcept {
+  if (!SPIRVAllowUnknownIntrinsics.hasValue())
+    return false;
+  const auto &IntrinsicPrefixList = SPIRVAllowUnknownIntrinsics.getValue();
+  StringRef IntrinsicName = II->getCalledOperand()->getName();
+  for (const auto Prefix : IntrinsicPrefixList) {
+    if (IntrinsicName.startswith(Prefix)) // Also true if `Prefix` is empty
+      return true;
+  }
+  return false;
+}
+
+bool TranslatorOpts::isSPIRVAllowUnknownIntrinsicsEnabled() const noexcept {
+  return SPIRVAllowUnknownIntrinsics.hasValue();
+}
+
+void TranslatorOpts::setSPIRVAllowUnknownIntrinsics(
+    TranslatorOpts::ArgList IntrinsicPrefixList) noexcept {
+  SPIRVAllowUnknownIntrinsics = IntrinsicPrefixList;
+}

lib/SPIRV/SPIRVWriter.cpp

@@ -2716,7 +2716,7 @@ SPIRVValue *LLVMToSPIRV::transIntrinsicInst(IntrinsicInst *II,
       return BM->addInstTemplate(OpSaveMemoryINTEL, BB, Ty);
     }
     BM->getErrorLog().checkError(
-        BM->isSPIRVAllowUnknownIntrinsicsEnabled(), SPIRVEC_InvalidFunctionCall,
+        BM->isUnknownIntrinsicAllowed(II), SPIRVEC_InvalidFunctionCall,
         toString(II) + "\nTranslation of llvm.stacksave intrinsic requires "
                        "SPV_INTEL_variable_length_array extension or "
                        "-spirv-allow-unknown-intrinsics option.");
@@ -2730,7 +2730,7 @@ SPIRVValue *LLVMToSPIRV::transIntrinsicInst(IntrinsicInst *II,
                                  nullptr);
     }
     BM->getErrorLog().checkError(
-        BM->isSPIRVAllowUnknownIntrinsicsEnabled(), SPIRVEC_InvalidFunctionCall,
+        BM->isUnknownIntrinsicAllowed(II), SPIRVEC_InvalidFunctionCall,
         toString(II) + "\nTranslation of llvm.restore intrinsic requires "
                        "SPV_INTEL_variable_length_array extension or "
                        "-spirv-allow-unknown-intrinsics option.");
@@ -2753,7 +2753,7 @@ SPIRVValue *LLVMToSPIRV::transIntrinsicInst(IntrinsicInst *II,
       return transValue(ConstantInt::getFalse(II->getType()), BB, false);
   }
   default:
-    if (BM->isSPIRVAllowUnknownIntrinsicsEnabled())
+    if (BM->isUnknownIntrinsicAllowed(II))
       return BM->addCallInst(
           transFunctionDecl(II->getCalledFunction()),
           transArguments(II, BB,

lib/SPIRV/libSPIRV/SPIRVModule.h

@@ -486,6 +486,10 @@ class SPIRVModule {
     return TranslationOpts.getFPContractMode();
   }
 
+  bool isUnknownIntrinsicAllowed(llvm::IntrinsicInst *II) const noexcept {
+    return TranslationOpts.isUnknownIntrinsicAllowed(II);
+  }
+
   bool isSPIRVAllowUnknownIntrinsicsEnabled() const noexcept {
     return TranslationOpts.isSPIRVAllowUnknownIntrinsicsEnabled();
   }

test/transcoding/AllowIntrinsics.ll

@@ -1,27 +1,52 @@
-; The test checks command-line option for the translator
-; which will allow to represent unknown llvm intrinsics as external function call in SPIR-V.
+; The test checks command-line option for the translator which allows to represent
+; unknown intrinsics as external function calls in SPIR-V.
 ; RUN: llvm-as %s -o %t.bc
 ; RUN: llvm-spirv -spirv-allow-unknown-intrinsics %t.bc -o %t.spv
 ; RUN: spirv-val %t.spv
 ; RUN: llvm-spirv %t.spv -to-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
 ; RUN: llvm-spirv -r %t.spv -o %t.rev.bc
 ; RUN: llvm-dis < %t.rev.bc | FileCheck %s --check-prefix=CHECK-LLVM
 
+; Check that passing a prefix list into the option works/fails as expected.
+;
+; Positive cases:
+; RUN: llvm-spirv -spirv-allow-unknown-intrinsics=llvm. %t.bc
+; RUN: llvm-spirv -spirv-allow-unknown-intrinsics=llvm.,random.prefix %t.bc
+;
+; Positive cases (identical to "no setting at all"):
+; RUN: llvm-spirv -spirv-allow-unknown-intrinsics= %t.bc
+; RUN: llvm-spirv -spirv-allow-unknown-intrinsics="" %t.bc
+; RUN: llvm-spirv -spirv-allow-unknown-intrinsics=random.prefix, %t.bc
+;
+; Negative cases:
+; RUN: not llvm-spirv -spirv-allow-unknown-intrinsics=llvm.some.custom %t.bc 2>&1 \
+; RUN: | FileCheck %s --check-prefix=CHECK-FAIL-READCYCLE
+; CHECK-FAIL-READCYCLE: InvalidFunctionCall: Unexpected llvm intrinsic:
+; CHECK-FAIL-READCYCLE-NEXT: llvm.readcyclecounter
+; RUN: not llvm-spirv -spirv-allow-unknown-intrinsics=llvm.readcyclecounter %t.bc 2>&1 \
+; RUN: | FileCheck %s --check-prefix=CHECK-FAIL-CUSTOM
+; CHECK-FAIL-CUSTOM: InvalidFunctionCall: Unexpected llvm intrinsic:
+; CHECK-FAIL-CUSTOM-NEXT: llvm.some.custom.intrinsic
+; RUN: not llvm-spirv -spirv-allow-unknown-intrinsics=non.llvm.1,non,llvm.2 %t.bc
+
 ; Note: This test used to call llvm.fma, but that is now traslated correctly.
 
 ; CHECK-LLVM: declare i64 @llvm.readcyclecounter()
 ; CHECK-SPIRV: LinkageAttributes "llvm.readcyclecounter" Import
+; CHECK-LLVM: declare i64 @llvm.some.custom.intrinsic()
 target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
 target triple = "spir64"
 
 ; Function Attrs: nounwind
 define spir_func void @foo() #0 {
 entry:
   %0 = call i64 @llvm.readcyclecounter()
+  %1 = call i64 @llvm.some.custom.intrinsic()
   ret void
 }
 
 declare i64 @llvm.readcyclecounter()
+declare i64 @llvm.some.custom.intrinsic()
 
 attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-realign-stack" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
 

test/transcoding/SPV_INTEL_vector_compute/decoration_byte_offset.ll

@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.bc
-; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics
+; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics=llvm.genx
 ; RUN: llvm-spirv %t.spv -o %t.spt --to-text
 ; RUN: llvm-spirv -r %t.spv -o %t.bc
 ; RUN: llvm-dis %t.bc -o %t.ll

test/transcoding/SPV_INTEL_vector_compute/decoration_simt_call.ll

@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.bc
-; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics
+; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics=llvm.genx
 ; RUN: llvm-spirv %t.spv -o %t.spt --to-text
 ; RUN: llvm-spirv -r %t.spv -o %t.bc
 ; RUN: llvm-dis %t.bc -o %t.ll

test/transcoding/SPV_INTEL_vector_compute/decoration_single_element_vector.ll

@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.bc
-; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics
+; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics=llvm.genx
 ; RUN: llvm-spirv %t.spv -o %t.spt --to-text
 ; RUN: llvm-spirv -r %t.spv -o %t.bc
 ; RUN: llvm-dis %t.bc -o %t.ll

test/transcoding/SPV_INTEL_vector_compute/decoration_volatile.ll

@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.bc
-; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics
+; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-allow-unknown-intrinsics=llvm.genx
 ; RUN: llvm-spirv %t.spv -o %t.spt --to-text
 ; RUN: llvm-spirv -r %t.spv -o %t.bc
 ; RUN: llvm-dis %t.bc -o %t.ll

test/transcoding/SPV_INTEL_vector_compute/extension_vector_compute_stability.ll

@@ -1,5 +1,5 @@
 ; RUN: llvm-as %s -o %t.bc
-; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute,+SPV_KHR_float_controls,+SPV_INTEL_float_controls2 --spirv-allow-unknown-intrinsics
+; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute,+SPV_KHR_float_controls,+SPV_INTEL_float_controls2 --spirv-allow-unknown-intrinsics=llvm.genx
 ; RUN: llvm-spirv -r %t.spv -o %t.bc
 ; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute,+SPV_KHR_float_controls,+SPV_INTEL_float_controls2 --spirv-allow-unknown-intrinsics
 ; RUN: llvm-spirv -r %t.spv -o %t.bc

tools/llvm-spirv/llvm-spirv.cpp

@@ -113,6 +113,14 @@ static cl::list<std::string>
            cl::value_desc("+SPV_extenstion1_name,-SPV_extension2_name"),
            cl::ValueRequired);
 
+static cl::list<std::string> SPIRVAllowUnknownIntrinsics(
+    "spirv-allow-unknown-intrinsics", cl::CommaSeparated,
+    cl::desc("Unknown intrinsics that begin with any prefix from the "
+             "comma-separated input list will be translated as external "
+             "function calls in SPIR-V.\nLeaving any prefix unspecified "
+             "(default) would naturally allow all unknown intrinsics"),
+    cl::value_desc("intrinsic_prefix_1,intrinsic_prefix_2"), cl::ValueOptional);
+
 static cl::opt<bool> SPIRVGenKernelArgNameMD(
     "spirv-gen-kernel-arg-name-md", cl::init(false),
     cl::desc("Enable generating OpenCL kernel argument name "
@@ -178,11 +186,6 @@ static cl::opt<SPIRV::FPContractMode> FPCMode(
             SPIRV::FPContractMode::Fast, "fast",
             "allow all operations to be contracted for all entry points")));
 
-cl::opt<bool> SPIRVAllowUnknownIntrinsics(
-    "spirv-allow-unknown-intrinsics", cl::init(false),
-    cl::desc("Unknown LLVM intrinsics will be translated as external function "
-             "calls in SPIR-V"));
-
 static cl::opt<bool> SPIRVAllowExtraDIExpressions(
     "spirv-allow-extra-diexpressions", cl::init(false),
     cl::desc("Allow DWARF operations not listed in the OpenCL.DebugInfo.100 "
@@ -543,6 +546,14 @@ bool parseSpecConstOpt(llvm::StringRef SpecConstStr,
   return false;
 }
 
+static void parseAllowUnknownIntrinsicsOpt(SPIRV::TranslatorOpts &Opts) {
+  SPIRV::TranslatorOpts::ArgList PrefixList;
+  for (const auto &Prefix : SPIRVAllowUnknownIntrinsics) {
+    PrefixList.push_back(Prefix);
+  }
+  Opts.setSPIRVAllowUnknownIntrinsics(PrefixList);
+}
+
 int main(int Ac, char **Av) {
   EnablePrettyStackTrace();
   sys::PrintStackTraceOnErrorSignal(Av[0]);
@@ -589,7 +600,7 @@ int main(int Ac, char **Av) {
           << "Note: --spirv-allow-unknown-intrinsics option ignored as it only "
              "affects translation from LLVM IR to SPIR-V";
     } else {
-      Opts.setSPIRVAllowUnknownIntrinsicsEnabled(SPIRVAllowUnknownIntrinsics);
+      parseAllowUnknownIntrinsicsOpt(Opts);
     }
   }