Skip to content

Commit

Permalink
spirv-as: Assume target from spvasm text (#5893)
Browse files Browse the repository at this point in the history 
* spirv-as: Assume target from spvasm text

Assume the desired SPIR-V target version when the command line option is
not provided. This is especially important when the source being
assembled does not match the default (latest version), because it may
create an invalid SPIR-V binary result.

Also, clarify the wording for where a copy of spirv-headers is expected
(to match expected capitalization).

* fix formatting

* Add unit tests, static asserts on supported SPIR-V versions

Rework the algorithm so it accepts earlier termination of the version
string. That made testing easier and more intuitive.

* Fix compilation problems

---------

Co-authored-by: mmoult <30711895+mmoult@users.noreply.github.com>
  • Loading branch information
@dneto0 @mmoult
dneto0 and mmoult authored Nov 26, 2024
1 parent f3c4a50 commit a9d884e
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 53 deletions.
2 changes: 1 addition & 1 deletion external/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ if (IS_DIRECTORY ${SPIRV_HEADER_DIR})
endif()
else()
message(FATAL_ERROR
"SPIRV-Headers was not found - please checkout a copy under external/.")
"SPIRV-Headers was not found - please checkout a copy at external/spirv-headers.")
endif()

if (NOT ${SPIRV_SKIP_TESTS})
Expand Down
120 changes: 93 additions & 27 deletions source/spirv_target_env.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@

#include "source/spirv_target_env.h"

#include <array>
#include <cassert>
#include <cctype>
#include <cstring>
#include <string>
#include <utility>

#include "source/latest_version_spirv_header.h"
#include "source/spirv_constant.h"
#include "spirv-tools/libspirv.h"

Expand Down Expand Up @@ -128,32 +130,43 @@ uint32_t spvVersionForTargetEnv(spv_target_env env) {
return SPV_SPIRV_VERSION_WORD(0, 0);
}

static const std::pair<const char*, spv_target_env> spvTargetEnvNameMap[] = {
{"vulkan1.1spv1.4", SPV_ENV_VULKAN_1_1_SPIRV_1_4},
{"vulkan1.0", SPV_ENV_VULKAN_1_0},
{"vulkan1.1", SPV_ENV_VULKAN_1_1},
{"vulkan1.2", SPV_ENV_VULKAN_1_2},
{"vulkan1.3", SPV_ENV_VULKAN_1_3},
{"spv1.0", SPV_ENV_UNIVERSAL_1_0},
{"spv1.1", SPV_ENV_UNIVERSAL_1_1},
{"spv1.2", SPV_ENV_UNIVERSAL_1_2},
{"spv1.3", SPV_ENV_UNIVERSAL_1_3},
{"spv1.4", SPV_ENV_UNIVERSAL_1_4},
{"spv1.5", SPV_ENV_UNIVERSAL_1_5},
{"spv1.6", SPV_ENV_UNIVERSAL_1_6},
{"opencl1.2embedded", SPV_ENV_OPENCL_EMBEDDED_1_2},
{"opencl1.2", SPV_ENV_OPENCL_1_2},
{"opencl2.0embedded", SPV_ENV_OPENCL_EMBEDDED_2_0},
{"opencl2.0", SPV_ENV_OPENCL_2_0},
{"opencl2.1embedded", SPV_ENV_OPENCL_EMBEDDED_2_1},
{"opencl2.1", SPV_ENV_OPENCL_2_1},
{"opencl2.2embedded", SPV_ENV_OPENCL_EMBEDDED_2_2},
{"opencl2.2", SPV_ENV_OPENCL_2_2},
{"opengl4.0", SPV_ENV_OPENGL_4_0},
{"opengl4.1", SPV_ENV_OPENGL_4_1},
{"opengl4.2", SPV_ENV_OPENGL_4_2},
{"opengl4.3", SPV_ENV_OPENGL_4_3},
{"opengl4.5", SPV_ENV_OPENGL_4_5},
// When a new SPIR-V version is released, update this table.
static_assert(spv::Version == 0x10600);
constexpr auto ordered_universal_envs = std::array<spv_target_env, 7>{
SPV_ENV_UNIVERSAL_1_0, SPV_ENV_UNIVERSAL_1_1, SPV_ENV_UNIVERSAL_1_2,
SPV_ENV_UNIVERSAL_1_3, SPV_ENV_UNIVERSAL_1_4, SPV_ENV_UNIVERSAL_1_5,
SPV_ENV_UNIVERSAL_1_6,
};

// When a new SPIR-V version is released, update this table.
static_assert(spv::Version == 0x10600);
inline constexpr std::pair<const char*, spv_target_env> spvTargetEnvNameMap[] =
{
{"vulkan1.1spv1.4", SPV_ENV_VULKAN_1_1_SPIRV_1_4},
{"vulkan1.0", SPV_ENV_VULKAN_1_0},
{"vulkan1.1", SPV_ENV_VULKAN_1_1},
{"vulkan1.2", SPV_ENV_VULKAN_1_2},
{"vulkan1.3", SPV_ENV_VULKAN_1_3},
{"spv1.0", SPV_ENV_UNIVERSAL_1_0},
{"spv1.1", SPV_ENV_UNIVERSAL_1_1},
{"spv1.2", SPV_ENV_UNIVERSAL_1_2},
{"spv1.3", SPV_ENV_UNIVERSAL_1_3},
{"spv1.4", SPV_ENV_UNIVERSAL_1_4},
{"spv1.5", SPV_ENV_UNIVERSAL_1_5},
{"spv1.6", SPV_ENV_UNIVERSAL_1_6},
{"opencl1.2embedded", SPV_ENV_OPENCL_EMBEDDED_1_2},
{"opencl1.2", SPV_ENV_OPENCL_1_2},
{"opencl2.0embedded", SPV_ENV_OPENCL_EMBEDDED_2_0},
{"opencl2.0", SPV_ENV_OPENCL_2_0},
{"opencl2.1embedded", SPV_ENV_OPENCL_EMBEDDED_2_1},
{"opencl2.1", SPV_ENV_OPENCL_2_1},
{"opencl2.2embedded", SPV_ENV_OPENCL_EMBEDDED_2_2},
{"opencl2.2", SPV_ENV_OPENCL_2_2},
{"opengl4.0", SPV_ENV_OPENGL_4_0},
{"opengl4.1", SPV_ENV_OPENGL_4_1},
{"opengl4.2", SPV_ENV_OPENGL_4_2},
{"opengl4.3", SPV_ENV_OPENGL_4_3},
{"opengl4.5", SPV_ENV_OPENGL_4_5},
};

bool spvParseTargetEnv(const char* s, spv_target_env* env) {
Expand All @@ -172,6 +185,59 @@ bool spvParseTargetEnv(const char* s, spv_target_env* env) {
return false;
}

bool spvReadEnvironmentFromText(const std::vector<char>& text,
spv_target_env* env) {
// Version is expected to match "; Version: 1.X"
// Version string must occur in header, that is, initial lines of comments
// Once a non-comment line occurs, the header has ended
for (std::size_t i = 0; i < text.size(); ++i) {
char c = text[i];

if (c == ';') {
// Try to match against the expected version string
constexpr const char* kVersionPrefix = "; Version: 1.";
constexpr const auto kPrefixLength = 13;
// 'minor_digit_pos' is the expected position of the version digit.
const auto minor_digit_pos = i + kPrefixLength;
if (minor_digit_pos >= text.size()) return false;

// Match the prefix.
auto j = 1;
for (; j < kPrefixLength; ++j) {
if (kVersionPrefix[j] != text[i + j]) break;
}
// j will match the prefix length if all characters before matched
if (j == kPrefixLength) {
// This expects only one digit in the minor number.
static_assert(((spv::Version >> 8) & 0xff) < 10);
char minor = text[minor_digit_pos];
char next_char =
minor_digit_pos + 1 < text.size() ? text[minor_digit_pos + 1] : 0;
if (std::isdigit(minor) && !std::isdigit(next_char)) {
const auto index = minor - '0';
assert(index >= 0);
if (static_cast<size_t>(index) < ordered_universal_envs.size()) {
*env = ordered_universal_envs[index];
return true;
}
}
}

// If no match, determine whether the header has ended (in which case,
// assumption has failed.)
// Skip until the next line.
i = j;
for (; i < text.size(); ++i) {
if (text[i] == '\n') break;
}
} else if (!std::isspace(c)) {
// Allow blanks, but end the search if we find something else.
break;
}
}
return false;
}

#define VULKAN_VER(MAJOR, MINOR) ((MAJOR << 22) | (MINOR << 12))
#define SPIRV_VER(MAJOR, MINOR) ((MAJOR << 16) | (MINOR << 8))

Expand Down
7 changes: 7 additions & 0 deletions source/spirv_target_env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#define SOURCE_SPIRV_TARGET_ENV_H_

#include <string>
#include <utility>
#include <vector>

#include "spirv-tools/libspirv.h"

Expand Down Expand Up @@ -46,4 +48,9 @@ std::string spvLogStringForEnv(spv_target_env env);
// occur to satisfy this limit.
std::string spvTargetEnvList(const int pad, const int wrap);

// Reads the target environment from the header comments of disassembly. Returns
// true if valid name found, false otherwise.
bool spvReadEnvironmentFromText(const std::vector<char>& text,
spv_target_env* env);

#endif // SOURCE_SPIRV_TARGET_ENV_H_
57 changes: 56 additions & 1 deletion test/target_env_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,5 +170,60 @@ INSTANTIATE_TEST_SUITE_P(
{VK(99, 0), SPV(1, 0), false, SPV_ENV_UNIVERSAL_1_0},
}));

} // namespace
// A test case for parsing the text header of disassembly.
struct ParseEnvInDisassemblyCase {
std::string text;
bool success; // Expect to successfully parse?
spv_target_env env; // The parsed environment, if successful.
};

using TargetParseEnvInDisassemblyTest =
::testing::TestWithParam<ParseEnvInDisassemblyCase>;

constexpr spv_target_env kSentinelEnv = SPV_ENV_OPENCL_2_2;

TEST_P(TargetParseEnvInDisassemblyTest, Samples) {
const std::string& text = GetParam().text;
const std::vector<char> text_vec(text.begin(), text.end());
spv_target_env got_env = kSentinelEnv;
bool parsed = spvReadEnvironmentFromText(text_vec, &got_env);
EXPECT_EQ(parsed, GetParam().success);
EXPECT_EQ(got_env, GetParam().env) << '"' << text << '"';
}

INSTANTIATE_TEST_SUITE_P(
TargetTextParsing, TargetParseEnvInDisassemblyTest,
ValuesIn(std::vector<ParseEnvInDisassemblyCase>{
{"; Version: 1.0", true, SPV_ENV_UNIVERSAL_1_0},
{"; Version: 1.1", true, SPV_ENV_UNIVERSAL_1_1},
{"; Version: 1.2", true, SPV_ENV_UNIVERSAL_1_2},
{"; Version: 1.3", true, SPV_ENV_UNIVERSAL_1_3},
{"; Version: 1.4", true, SPV_ENV_UNIVERSAL_1_4},
{"; Version: 1.5", true, SPV_ENV_UNIVERSAL_1_5},
{"; Version: 1.6", true, SPV_ENV_UNIVERSAL_1_6},
{"; Version: 1.7", false, kSentinelEnv},
{"; Version: 1.8", false, kSentinelEnv},
{"; Version: 1.9", false, kSentinelEnv},
{"; Version: 2.0", false, kSentinelEnv},

// Check trailing text
{"; Version: 1.1\n", true, SPV_ENV_UNIVERSAL_1_1},
{"; Version: 1.1\t", true, SPV_ENV_UNIVERSAL_1_1},
{"; Version: 1.1 ", true, SPV_ENV_UNIVERSAL_1_1},
{"; Version: 1.1x", true, SPV_ENV_UNIVERSAL_1_1},
// Not a digit.
{"; Version: 1.10", false, kSentinelEnv},

// Unexpected prefix
{";Version: 1.1", false, kSentinelEnv},

// Leading spaces
{" \t ; Version: 1.1", true, SPV_ENV_UNIVERSAL_1_1},
// Previous lines
{"; SPIR-V\n; Version: 1.1", true, SPV_ENV_UNIVERSAL_1_1},

// After a non-header line
{"OpCapability Shader\n; Version: 1.1", false, kSentinelEnv}}));

} // anonymous namespace
} // namespace spvtools
47 changes: 23 additions & 24 deletions tools/as/as.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include "tools/io.h"
#include "tools/util/flags.h"

static const auto kDefaultEnvironment = "spv1.6";
constexpr auto kDefaultTarget = SPV_ENV_UNIVERSAL_1_6;
static const std::string kHelpText =
R"(%s - Create a SPIR-V binary module from SPIR-V assembly text
Expand All @@ -48,12 +48,13 @@ is used.
)";

// clang-format off
FLAG_SHORT_bool( h, /* default_value= */ false, /* required= */ false);
FLAG_LONG_bool( help, /* default_value= */ false, /* required= */false);
FLAG_LONG_bool( version, /* default_value= */ false, /* required= */ false);
FLAG_LONG_bool( preserve_numeric_ids, /* default_value= */ false, /* required= */ false);
FLAG_SHORT_string(o, /* default_value= */ "", /* required= */ false);
FLAG_LONG_string( target_env, /* default_value= */ kDefaultEnvironment, /* required= */ false);
// flag name= default_value= required=
FLAG_SHORT_bool( h, false, false);
FLAG_LONG_bool( help, false, false);
FLAG_LONG_bool( version, false, false);
FLAG_LONG_bool( preserve_numeric_ids, false, false);
FLAG_SHORT_string(o, "", false);
FLAG_LONG_string( target_env, "", false);
// clang-format on

int main(int, const char** argv) {
Expand All @@ -68,17 +69,8 @@ int main(int, const char** argv) {
}

if (flags::version.value()) {
spv_target_env target_env;
bool success = spvParseTargetEnv(kDefaultEnvironment, &target_env);
assert(success && "Default environment should always parse.");
if (!success) {
fprintf(stderr,
"error: invalid default target environment. Please report this "
"issue.");
return 1;
}
printf("%s\n", spvSoftwareVersionDetailsString());
printf("Target: %s\n", spvTargetEnvDescription(target_env));
printf("Target: %s\n", spvTargetEnvDescription(kDefaultTarget));
return 0;
}

Expand All @@ -92,13 +84,6 @@ int main(int, const char** argv) {
options |= SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS;
}

spv_target_env target_env;
if (!spvParseTargetEnv(flags::target_env.value().c_str(), &target_env)) {
fprintf(stderr, "error: Unrecognized target env: %s\n",
flags::target_env.value().c_str());
return 1;
}

if (flags::positional_arguments.size() != 1) {
fprintf(stderr, "error: exactly one input file must be specified.\n");
return 1;
Expand All @@ -108,6 +93,20 @@ int main(int, const char** argv) {
std::vector<char> contents;
if (!ReadTextFile(inFile.c_str(), &contents)) return 1;

// Can only deduce target after the file has been read
spv_target_env target_env;
if (flags::target_env.value().empty()) {
if (!spvReadEnvironmentFromText(contents, &target_env)) {
// Revert to default version since deduction failed
target_env = kDefaultTarget;
}
} else if (!spvParseTargetEnv(flags::target_env.value().c_str(),
&target_env)) {
fprintf(stderr, "error: Unrecognized target env: %s\n",
flags::target_env.value().c_str());
return 1;
}

spv_binary binary;
spv_diagnostic diagnostic = nullptr;
spv_context context = spvContextCreate(target_env);
Expand Down

0 comments on commit a9d884e

Please sign in to comment.