Simplify map of SPIR-V numeric constants.

Previously, we used a union of (int64, float) to store the cached value.
However, the union-based code turned out to be more complex than just
type-punning the float's bits to an int via memcpy (which we needed to
do anyway). The union-based approach was also likely to be UB by the
letter of the law--we were creating float keys by storing into fFloat,
then checking the map by comparing equality on fInt.

Change-Id: I62c7ff4b5fab8bb1be8836c23f746ef254053b6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350957
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2021-01-06 18:41:40 -05:00 committed by Skia Commit-Bot
parent 9cfaa4ffa1
commit bdc3d3c60f
2 changed files with 19 additions and 29 deletions

View File

@ -2610,7 +2610,7 @@ SpvId SPIRVCodeGenerator::writeBoolLiteral(const BoolLiteral& b) {
}
SpvId SPIRVCodeGenerator::writeIntLiteral(const IntLiteral& i) {
ConstantValuePair key(i.value(), i.type().numberKind());
SPIRVNumberConstant key{i.value(), i.type().numberKind()};
auto [iter, newlyCreated] = fNumberConstants.insert({key, (SpvId)-1});
if (newlyCreated) {
SpvId result = this->nextId();
@ -2622,15 +2622,17 @@ SpvId SPIRVCodeGenerator::writeIntLiteral(const IntLiteral& i) {
}
SpvId SPIRVCodeGenerator::writeFloatLiteral(const FloatLiteral& f) {
ConstantValuePair key(f.value(), f.type().numberKind());
// Convert the float literal into its bit-representation.
float value = f.value();
uint32_t valueBits;
static_assert(sizeof(valueBits) == sizeof(value));
memcpy(&valueBits, &value, sizeof(value));
SPIRVNumberConstant key{valueBits, f.type().numberKind()};
auto [iter, newlyCreated] = fNumberConstants.insert({key, (SpvId)-1});
if (newlyCreated) {
SpvId result = this->nextId();
float value = f.value();
uint32_t valueBits;
static_assert(sizeof(valueBits) == sizeof(value));
memcpy(&valueBits, &value, sizeof(valueBits));
this->writeInstruction(SpvOpConstant, this->getType(f.type()), result, valueBits,
this->writeInstruction(SpvOpConstant, this->getType(f.type()), result, (SpvId) valueBits,
fConstantBuffer);
iter->second = result;
}

View File

@ -44,27 +44,15 @@
namespace SkSL {
union ConstantValue {
ConstantValue(SKSL_INT i)
: fInt(i) {
static_assert(sizeof(*this) == sizeof(SKSL_INT));
struct SPIRVNumberConstant {
bool operator==(const SPIRVNumberConstant& that) const {
return fValueBits == that.fValueBits &&
fKind == that.fKind;
}
ConstantValue(SKSL_FLOAT f) {
memset(this, 0, sizeof(*this));
fFloat = f;
}
bool operator==(const ConstantValue& other) const {
return fInt == other.fInt;
}
SKSL_INT fInt;
SKSL_FLOAT fFloat;
int64_t fValueBits; // contains either an SKSL_INT or zero-padded bits from an SKSL_FLOAT
SkSL::Type::NumberKind fKind;
};
using ConstantValuePair = std::pair<ConstantValue, SkSL::Type::NumberKind>;
struct SPIRVVectorConstant {
bool operator==(const SPIRVVectorConstant& that) const {
return fTypeId == that.fTypeId &&
@ -82,9 +70,9 @@ struct SPIRVVectorConstant {
namespace std {
template <>
struct hash<SkSL::ConstantValuePair> {
size_t operator()(const SkSL::ConstantValuePair& key) const {
return key.first.fInt ^ (int) key.second;
struct hash<SkSL::SPIRVNumberConstant> {
size_t operator()(const SkSL::SPIRVNumberConstant& key) const {
return key.fValueBits ^ (int)key.fKind;
}
};
@ -404,7 +392,7 @@ private:
SpvId fBoolTrue;
SpvId fBoolFalse;
std::unordered_map<ConstantValuePair, SpvId> fNumberConstants;
std::unordered_map<SPIRVNumberConstant, SpvId> fNumberConstants;
std::unordered_map<SPIRVVectorConstant, SpvId> fVectorConstants;
bool fSetupFragPosition;
// label of the current block, or 0 if we are not in a block