[PATCH vkd3d 5/6] vkd3d-shader/hlsl: Introduce specialized helpers for arithmetic operations.

[Giovanni Mascellani]

Hi,

Il 14/10/21 18:32, Zebediah Figura ha scritto:
> Yeah, but the ternary operator is a special case. I don't think it 
> should result in an HLSL_IR_EXPR. Probably we should synthesize a 
> temporary variable, store to it inside of a conditional, and load from it.

Why? I don't know in SM1, but at least in SM4 we have a nice opcode 
(MOVC) that does just what the ternary operator needs to do, in a way 
that is probably more efficient (or at least not less efficient) than 
doing an actual branch. Also, I have a few other patches in my queue 
that use MOVC for other things (like implementing casts from bool), so 
I'd like to have it in the IR (not really related to this patch, though).

> I'm still not convinced that casting in add_expr() is better than 
> casting in the caller, either.

 From my point of view it makes the code more declarative and therefore 
easier to read. However apparently we have different tastes, and I don't 
have a too strong opinion.

>> The add_*_last helpers behave similarly as their counterpart without
>> "_last", except that the last node in each instruction list is used
>> as an argument.
> 
> Yeah, I still don't like this name. "last" makes me think that the 
> instruction will be added to the end of a list, or something. "merge" or 
> "combine" would be a lot better, but it's a bit awkward for the unary 
> helpers. But then again maybe we don't need the unary helpers, and we 
> can just as easily just spell out node_from_list() inline. There are 
> only three such cases after all (and I guess they're not even all the 
> same kind of operation).
> 
> Or "parse", I guess, to symbolize it's done as part of parsing, although 
> that's not great either.

I don't like it either, but neither I like better one of your 
alternatives, nor I could find anything better. Among your suggestions I 
would pick "parse", but if you eventually settle for something else just 
tell me.

> I think at least splitting the "cast_types" part out of this patch would 
> make it easier to read.

Ok, will do.

Thanks again, Giovanni.