On Mon Jun 5 20:00:00 2023 +0000, Nikolay Sivov wrote:
I can spend time on that later, once it's decided how to proceed.
As you wish, but I won't review that patch until it's reviewable.
What needs to be reviewed is intrinsic_clip() and lower_texkill(), whether that conceptually correct, not renames or variable names. The rest of the diff, as you can see, is there to be able to build and test.
As for having a transform pass rather than immediately creating two
different codes, I don't have strong feelings. As I already said, I think it is more consistent with what we already do for the conditional operator to initially create an `if` block, and later possibly raising it to `DISCARD` (or whatever of its variants). That's also what @Mystral suggested and possibly what @zfigura suggested saying that we don't want the same transformation in both directions. But I'll leave to them to stress their point of view if they so wish. I could use a strong feeling exactly, but about important part of the patch. Native compiler does not collapse such if blocks, as far as I remember, nor does it use unconditional discard instruction. So far I see it only using discard_nz at all times. There is an option to do: clip -> 'if (arg<0) then discard;' -> for sm1 turn discard to texkill with initialized register. Not only this is unnecessarily more complicated, but it also does not match what happens for sm1, where clip is mapped directly to texkill. And then it will need additional work for get rid of conditional branching for SM1. It looks more like going out of the way to produce different output for no reason. So yes, @Mystral, @zfigura, I'd like another opinion on this - is it critical to have one DISCARD without an argument as a jump type that is used for all profiles?
I don't have a very strong opinion. Multiple discard types are fine; v1 of this patch was also fine. I mostly just want the IR to have the same semantics regardless of profile version.