Report spec constant types when they are queried with --spec-const-info (#1758)

Also allow binary float spec constant input by passing a hex value with
0x prefix.

Author: aarongreig

include/LLVMSPIRVLib.h

@@ -132,7 +132,11 @@ bool readSpirv(LLVMContext &C, const SPIRV::TranslatorOpts &Opts,
 /// \brief Partially load SPIR-V from the stream and decode only instructions
 /// needed to get information about specialization constants.
 /// \returns true if succeeds.
-using SpecConstInfoTy = std::pair<uint32_t, uint32_t>;
+struct SpecConstInfoTy {
+  uint32_t ID;
+  uint32_t Size;
+  std::string Type;
+};
 bool getSpecConstInfo(std::istream &IS,
                       std::vector<SpecConstInfoTy> &SpecConstInfo);
 

lib/SPIRV/SPIRVReader.cpp

@@ -4574,7 +4574,42 @@ bool llvm::getSpecConstInfo(std::istream &IS,
       if (C->hasDecorate(DecorationSpecId, 0, &SpecConstIdLiteral)) {
         SPIRVType *Ty = C->getType();
         uint32_t SpecConstSize = Ty->isTypeBool() ? 1 : Ty->getBitWidth() / 8;
-        SpecConstInfo.emplace_back(SpecConstIdLiteral, SpecConstSize);
+        std::string TypeString = "";
+        if (Ty->isTypeBool()) {
+          TypeString = "i1";
+        } else if (Ty->isTypeInt()) {
+          switch (SpecConstSize) {
+          case 1:
+            TypeString = "i8";
+            break;
+          case 2:
+            TypeString = "i16";
+            break;
+          case 4:
+            TypeString = "i32";
+            break;
+          case 8:
+            TypeString = "i64";
+            break;
+          }
+        } else if (Ty->isTypeFloat()) {
+          switch (SpecConstSize) {
+          case 2:
+            TypeString = "f16";
+            break;
+          case 4:
+            TypeString = "f32";
+            break;
+          case 8:
+            TypeString = "f64";
+            break;
+          }
+        }
+        if (TypeString == "")
+          return false;
+
+        SpecConstInfo.emplace_back(
+            SpecConstInfoTy({SpecConstIdLiteral, SpecConstSize, TypeString}));
       }
       break;
     }

test/SpecConstants/OpSpecConstant.spvasm

@@ -3,17 +3,17 @@
 
 ; RUN: llvm-spirv -spec-const-info %t.spv | FileCheck %s --check-prefix=CHECK-INFO
 ; CHECK-INFO: Number of scalar specialization constants in the module = 11
-; CHECK-INFO: Spec const id = 101, size in bytes = 1
-; CHECK-INFO: Spec const id = 102, size in bytes = 2
-; CHECK-INFO: Spec const id = 103, size in bytes = 4
-; CHECK-INFO: Spec const id = 104, size in bytes = 8
-; CHECK-INFO: Spec const id = 105, size in bytes = 1
-; CHECK-INFO: Spec const id = 106, size in bytes = 1
-; CHECK-INFO: Spec const id = 107, size in bytes = 1
-; CHECK-INFO: Spec const id = 108, size in bytes = 2
-; CHECK-INFO: Spec const id = 109, size in bytes = 4
-; CHECK-INFO: Spec const id = 110, size in bytes = 8
-; CHECK-INFO: Spec const id = 111, size in bytes = 8
+; CHECK-INFO: Spec const id = 101, size in bytes = 1, type = i8
+; CHECK-INFO: Spec const id = 102, size in bytes = 2, type = i16
+; CHECK-INFO: Spec const id = 103, size in bytes = 4, type = i32
+; CHECK-INFO: Spec const id = 104, size in bytes = 8, type = i64
+; CHECK-INFO: Spec const id = 105, size in bytes = 1, type = i8
+; CHECK-INFO: Spec const id = 106, size in bytes = 1, type = i1
+; CHECK-INFO: Spec const id = 107, size in bytes = 1, type = i1
+; CHECK-INFO: Spec const id = 108, size in bytes = 2, type = f16
+; CHECK-INFO: Spec const id = 109, size in bytes = 4, type = f32
+; CHECK-INFO: Spec const id = 110, size in bytes = 8, type = f64
+; CHECK-INFO: Spec const id = 111, size in bytes = 8, type = f64
 
 ; Negative tests for the command line option.
 ; RUN: not llvm-spirv -r -emit-opaque-pointers %t.spv -spec-const "1234" 2>&1 | FileCheck %s --check-prefix=CHECK-FORMAT
@@ -46,6 +46,9 @@
 ; RUN: llvm-spirv -r -emit-opaque-pointers -o - %t.spv | llvm-dis | FileCheck %s --check-prefix=CHECK-DEFAULT
 ; RUN: llvm-spirv -r -emit-opaque-pointers -spec-const "101:i8:42 102:i16:22 103:i32:33 104:i64:44 105:i8:0 106:i1:0 107:i1:1 108:f16:5.5 109:f32:6.6 110:f64:7.7 111:f64:8.8 101:i8:11" -o - %t.spv | llvm-dis | FileCheck %s --check-prefix=CHECK-SPEC
 
+; RUN: llvm-spirv -r -emit-opaque-pointers -o - %t.spv | llvm-dis | FileCheck %s --check-prefix=CHECK-DEFAULT
+; RUN: llvm-spirv -r -emit-opaque-pointers -spec-const "108:f16:0x1234 109:f32:0x12345678 110:f64:0x12345678ABCDEF00" -o - %t.spv | llvm-dis | FileCheck %s --check-prefix=CHECK-HEX
+
 ; CHECK-DEFAULT: @bt = addrspace(2) constant i1 true, align 1
 ; CHECK-DEFAULT: @bf = addrspace(2) constant i1 false, align 1
 ; CHECK-DEFAULT: @c = addrspace(2) constant i8 97, align 1
@@ -68,6 +71,11 @@
 ; CHECK-SPEC: @d = addrspace(2) constant double 7.700000e+00, align 8
 ; CHECK-SPEC: @ss = addrspace(1) global %struct.S { i8 0, double 8.800000e+00 }, align 8
 
+; CHECK-HEX: @h = addrspace(2) constant half 0xH1234, align 2
+; Float literals are represented as doubles, so 0x12345678 becomes the value below.
+; CHECK-HEX: @f = addrspace(2) constant float 0x3A468ACF00000000, align 4
+; CHECK-HEX: @d = addrspace(2) constant double 0x12345678ABCDEF00, align 8
+
 ; SPIR-V
 ; Version: 1.0
 ; Generator: Khronos LLVM/SPIR-V Translator; 14

test/lit.cfg.py

@@ -50,7 +50,7 @@
 
 config.substitutions.append(('%PATH%', config.environment['PATH']))
 
-tool_dirs = [config.llvm_tools_dir, config.llvm_spirv_dir]
+tool_dirs = [config.llvm_spirv_dir, config.llvm_tools_dir]
 
 tools = ['llvm-as', 'llvm-dis', 'llvm-spirv', 'not']
 if not config.spirv_skip_debug_info_tests:

tools/llvm-spirv/llvm-spirv.cpp

@@ -75,6 +75,7 @@
 #include <map>
 #include <memory>
 #include <set>
+#include <sstream>
 #include <string>
 
 #define DEBUG_TYPE "spirv"
@@ -183,6 +184,8 @@ static cl::opt<std::string> SpecConst(
              "SPIR-V module.\n"
              "The list of valid ids is available via -spec-const-info option.\n"
              "For duplicate ids the later one takes precedence.\n"
+             "Float values may be represented in decimal or hexadecimal, hex "
+             "values must be preceded by 0x.\n"
              "Supported types are: i1, i8, i16, i32, i64, f16, f32, f64.\n"),
     cl::value_desc("id1:type1:value1 id2:type2:value2 ..."));
 
@@ -552,9 +555,9 @@ bool parseSpecConstOpt(llvm::StringRef SpecConstStr,
              << "\" must be a 32-bit unsigned integer\n";
       return true;
     }
-    auto It = std::find_if(
-        SpecConstInfo.begin(), SpecConstInfo.end(),
-        [=](SpecConstInfoTy Info) { return Info.first == SpecId; });
+    auto It =
+        std::find_if(SpecConstInfo.begin(), SpecConstInfo.end(),
+                     [=](SpecConstInfoTy Info) { return Info.ID == SpecId; });
     if (It == SpecConstInfo.end()) {
       errs() << "Error: CL_INVALID_SPEC_ID. \"" << Option << "\": There is no "
              << "specialization constant with id = " << SpecId
@@ -572,11 +575,11 @@ bool parseSpecConstOpt(llvm::StringRef SpecConstStr,
         return true;
       }
       size_t Size = Width < 8 ? 1 : Width / 8;
-      if (Size != It->second) {
+      if (Size != It->Size) {
         errs() << "Error: CL_INVALID_VALUE. In \"" << Option << "\": Size of "
                << "type i" << Width << " (" << Size << " bytes) "
                << "does not match the size of the specialization constant "
-               << "in the module (" << It->second << " bytes)\n";
+               << "in the module (" << It->Size << " bytes)\n";
         return true;
       }
       APInt Value;
@@ -611,21 +614,29 @@ bool parseSpecConstOpt(llvm::StringRef SpecConstStr,
         return true;
       }
       APFloat Value(*FS);
-      Expected<APFloat::opStatus> StatusOrErr =
-          Value.convertFromString(Params[2], APFloat::rmNearestTiesToEven);
-      if (!StatusOrErr) {
-        return true;
-      }
-      // It's ok to have inexact conversion from decimal representation.
-      APFloat::opStatus Status = *StatusOrErr;
-      if (Status & ~APFloat::opInexact) {
-        errs() << "Error: Invalid value for '-" << SpecConst.ArgStr
-               << "' option! In \"" << Option << "\": can't convert \""
-               << Params[2] << "\" to " << Width
-               << "-bit floating point number\n";
-        return true;
+      if (Params[2].find("0x") != StringRef::npos) {
+        std::stringstream paramStream;
+        paramStream << std::hex << Params[2].data();
+        uint64_t specVal = 0;
+        paramStream >> specVal;
+        Opts.setSpecConst(SpecId, specVal);
+      } else {
+        Expected<APFloat::opStatus> StatusOrErr =
+            Value.convertFromString(Params[2], APFloat::rmNearestTiesToEven);
+        if (!StatusOrErr) {
+          return true;
+        }
+        // It's ok to have inexact conversion from decimal representation.
+        APFloat::opStatus Status = *StatusOrErr;
+        if (Status & ~APFloat::opInexact) {
+          errs() << "Error: Invalid value for '-" << SpecConst.ArgStr
+                 << "' option! In \"" << Option << "\": can't convert \""
+                 << Params[2] << "\" to " << Width
+                 << "-bit floating point number\n";
+          return true;
+        }
+        Opts.setSpecConst(SpecId, Value.bitcastToAPInt().getZExtValue());
       }
-      Opts.setSpecConst(SpecId, Value.bitcastToAPInt().getZExtValue());
     } else {
       errs() << "Error: Invalid type for '-" << SpecConst.ArgStr
              << "' option! In \"" << Option << "\": \"" << Params[1]
@@ -759,8 +770,9 @@ int main(int Ac, char **Av) {
     std::cout << "Number of scalar specialization constants in the module = "
               << SpecConstInfo.size() << "\n";
     for (auto &SpecConst : SpecConstInfo)
-      std::cout << "Spec const id = " << SpecConst.first
-                << ", size in bytes = " << SpecConst.second << "\n";
+      std::cout << "Spec const id = " << SpecConst.ID
+                << ", size in bytes = " << SpecConst.Size
+                << ", type = " << SpecConst.Type << "\n";
   }
   return 0;
 }