From: "Francisco Casas" Subject: Re: [PATCH vkd3d] vkd3d-shader/hlsl: Handle constant value operations on a different file. Message-Id: Date: Tue, 04 Jan 2022 16:59:36 +0000 In-Reply-To: <3a4f11c7-f56d-1690-1860-a3085b18467d@codeweavers.com> References: <3a4f11c7-f56d-1690-1860-a3085b18467d@codeweavers.com> <20220103145733.104047-1-fcasas@codeweavers.com> January 4, 2022 11:36 AM, "Giovanni Mascellani" wrote: > Hi, > > On 03/01/22 15:57, Francisco Casas wrote: > >> On constant folding, first switch on the op type, handling each >> operation using a function defined on hlsl_constant_ops.c. > > Creating a new file makes sense to me, but while you're doing it it also makes sense to copy there > the whole implementation of fold_constants(), doesn't it? It logically related to > hlsl_constant_ops.c, and it also spares you adding a lot of functions to the header file (also, if > you do that you probably have only one function to add to the header, fold_constants() itself, and > you can put it in hlsl.h instead of creating a new file). > That sounds like a very good idea to me! > While I don't have any specific objection against it, is there a particular reason for inverting > the switch on the type and on the operation? > Zebediah mentioned that in the future we may want to add operations that aren't typed. Also, I think that splitting the problem op-wise allows for a better organization since there are fairly more ops than types. >> Each of these operations switches on the data type, if needed. > > As I just said on the other thread (where I replied before noticing this one), watch out for > undefined behavior that some operations entail. > Noted. The only problem with (+), (*), and (-), I can think of is signed integer overflow. I am not sure yet if implicit castings in C may result in undefined behaviors, I must check.