Skip to content

Commit d9d83d0

Browse files
s-perronGreg Roth
authored andcommitted
Fix two issues found in our internal build (#5002)
1. The StringRef in the `stringLiterals` in the `SpirvBuilder` can have a dangling reference causing undefined behaviour. 2. Two symbols can have the same name, so when we sort a vector of symbols by name, we want to use std::stable_sort to make sure we get deterministic behaviour. (cherry picked from commit 7c4927a)
1 parent c8603e4 commit d9d83d0

2 files changed

Lines changed: 5 additions & 5 deletions

File tree

tools/clang/include/clang/SPIRV/SpirvBuilder.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ namespace spirv {
2626
struct StringMapInfo {
2727
static inline std::string getEmptyKey() { return ""; }
2828
static inline std::string getTombstoneKey() { return ""; }
29-
static unsigned getHashValue(const std::string Val) {
29+
static unsigned getHashValue(const std::string& Val) {
3030
return llvm::hash_combine(Val);
3131
}
32-
static bool isEqual(const std::string LHS, const std::string RHS) {
32+
static bool isEqual(const std::string& LHS, const std::string& RHS) {
3333
// Either both are null, or both should have the same underlying type.
3434
return LHS == RHS;
3535
}
@@ -851,7 +851,7 @@ class SpirvBuilder {
851851
// kept track of separately. This is because the empty string is used
852852
// as the EmptyKey and TombstoneKey for the map, prohibiting insertion
853853
// of the empty string as a contained value.
854-
llvm::DenseMap<llvm::StringRef, SpirvString *, StringMapInfo> stringLiterals;
854+
llvm::DenseMap<std::string, SpirvString *, StringMapInfo> stringLiterals;
855855
SpirvString *emptyString;
856856

857857
/// Mapping of CTBuffers including matrix 1xN with FXC memory layout to their

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,7 +2095,7 @@ bool DeclResultIdMapper::finalizeStageIOLocations(bool forInput) {
20952095
// If alphabetical ordering was requested, sort by semantic string.
20962096
if (spirvOptions.stageIoOrder == "alpha") {
20972097
// Sort stage input/output variables alphabetically
2098-
std::sort(vars.begin(), vars.end(),
2098+
std::stable_sort(vars.begin(), vars.end(),
20992099
[](const StageVar *a, const StageVar *b) {
21002100
return a->getSemanticStr() < b->getSemanticStr();
21012101
});
@@ -2117,7 +2117,7 @@ bool DeclResultIdMapper::finalizeStageIOLocations(bool forInput) {
21172117
// alphabetical ordering.
21182118
if ((!forInput && spvContext.isHS()) || (forInput && spvContext.isDS())) {
21192119
// Sort stage input/output variables alphabetically
2120-
std::sort(vars.begin(), vars.end(),
2120+
std::stable_sort(vars.begin(), vars.end(),
21212121
[](const StageVar *a, const StageVar *b) {
21222122
return a->getSemanticStr() < b->getSemanticStr();
21232123
});

0 commit comments

Comments
 (0)