From 929e960d2f81ff8ba0a7b979d67b955e2c4ba0e0 Mon Sep 17 00:00:00 2001 From: ravi688 Date: Sun, 16 Jun 2024 12:06:59 +0530 Subject: [PATCH 1/2] [BugFix] Do not generate any conversion for 8-bit and 16-bit types if it is an identity conversion This change affects the following type of shaders (which does identity conversion for 8/16 bit types): ``` layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in; layout(binding = 0) readonly buffer A { TYPE data_a[]; }; layout(binding = 1) writeonly buffer D { TYPE data_d[]; }; void main() { const uint i = gl_GlobalInvocationID.x; data_d[i] = TYPE(data_a[i]); } ``` Earlier (before this fix), it generated incorrect SPIR-V convert instructions. --- glslang/MachineIndependent/ParseHelper.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 98a7d26883..f6aca2b83c 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -8540,7 +8540,8 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T basicOp = EOpConstructFloat16; // 8/16-bit storage extensions don't support constructing composites of 8/16-bit types, // so construct a 32-bit type and convert - if (!intermediate.getArithemeticFloat16Enabled()) { + // and do not generate any conversion if it is an identity conversion, i.e. float16_t( var) + if (!intermediate.getArithemeticFloat16Enabled() && (node->getBasicType() != EbtFloat16)) { TType tempType(EbtFloat, EvqTemporary, type.getVectorSize()); newNode = node; if (tempType != newNode->getType()) { @@ -8563,7 +8564,8 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T basicOp = EOpConstructInt8; // 8/16-bit storage extensions don't support constructing composites of 8/16-bit types, // so construct a 32-bit type and convert - if (!intermediate.getArithemeticInt8Enabled()) { + // and do not generate any conversion if it is an identity conversion, i.e. int8_t( var) + if (!intermediate.getArithemeticInt8Enabled() && (node->getBasicType() != EbtInt8)) { TType tempType(EbtInt, EvqTemporary, type.getVectorSize()); newNode = node; if (tempType != newNode->getType()) { @@ -8586,7 +8588,8 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T basicOp = EOpConstructUint8; // 8/16-bit storage extensions don't support constructing composites of 8/16-bit types, // so construct a 32-bit type and convert - if (!intermediate.getArithemeticInt8Enabled()) { + // and do not generate any conversion if it is an identity conversion, i.e. uint8_t( var) + if (!intermediate.getArithemeticInt8Enabled() && (node->getBasicType() != EbtUint8)) { TType tempType(EbtUint, EvqTemporary, type.getVectorSize()); newNode = node; if (tempType != newNode->getType()) { @@ -8609,7 +8612,8 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T basicOp = EOpConstructInt16; // 8/16-bit storage extensions don't support constructing composites of 8/16-bit types, // so construct a 32-bit type and convert - if (!intermediate.getArithemeticInt16Enabled()) { + // and do not generate any conversion if it is an identity conversion, i.e. int16_t( var) + if (!intermediate.getArithemeticInt16Enabled() && (node->getBasicType() != EbtInt16)) { TType tempType(EbtInt, EvqTemporary, type.getVectorSize()); newNode = node; if (tempType != newNode->getType()) { @@ -8632,7 +8636,8 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T basicOp = EOpConstructUint16; // 8/16-bit storage extensions don't support constructing composites of 8/16-bit types, // so construct a 32-bit type and convert - if (!intermediate.getArithemeticInt16Enabled()) { + // and do not generate any conversion if it is an identity conversion, i.e. uint16_t( var) + if (!intermediate.getArithemeticInt16Enabled() && (node->getBasicType() != EbtUint16)) { TType tempType(EbtUint, EvqTemporary, type.getVectorSize()); newNode = node; if (tempType != newNode->getType()) { From 0c427567957c1ae244343f307716945cd2bdf351 Mon Sep 17 00:00:00 2001 From: ravi688 Date: Wed, 19 Jun 2024 22:26:07 +0530 Subject: [PATCH 2/2] [No Functional Change] Added copyright message (as per CLA) --- glslang/MachineIndependent/ParseHelper.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index f6aca2b83c..c5f0e63f14 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -4,6 +4,7 @@ // Copyright (C) 2015-2018 Google, Inc. // Copyright (C) 2017, 2019 ARM Limited. // Modifications Copyright (C) 2020 Advanced Micro Devices, Inc. All rights reserved. +// Modifications Copyright (C) 2024 Ravi Prakash Singh. // // All rights reserved. //