[PATCH vkd3d 2/5] vkd3d-shader/hlsl: Perform a copy propagation pass.

Matteo Bruni matteo.mystral at gmail.com
Thu Nov 11 05:14:00 CST 2021


On Thu, Nov 11, 2021 at 11:40 AM Giovanni Mascellani
<gmascellani at codeweavers.com> wrote:
>
> Hi,
>
> On 11/11/21 11:18, Matteo Bruni wrote:
> > Yes. An ideal comment says why you're doing something, not what. The
> > latter should be clear from the code; if you feel you need a comment
> > to explain the nooks and crannies of some code path in detail, chances
> > are that the code itself needs some more thought.
>
> This, as a blanket statement, seems a bit excessive to me. As I said,
> when reading code in places like user32 and winex11.drv I'd be very
> happy to have comments, even hard to read or getting in the specific
> details of something. Or, as I meant my comment to be initially,
> describing what data structures are supposed to represent.

I accept that in practice sometimes a comment explaining at a high
level what's happening can be good, especially if there is some
non-trivial algorithm involved. Same for comments on some very
specific technicality of the code in question. Those cases should be
the exception though.

> That said, the revised patch set that I sent two seconds before
> receiving this email should have been improved on that side (and,
> hopefully, many other).
>
> > I think just hardcoding an array of 4 for values (and getting rid of
> > struct copy_propagation_value altogether) would make things quite a
> > bit nicer.
>
> Notice that variables can have more than four components. Matrices can
> have up to 16 and arrays even more.

Right, but we probably don't want or need to do copy propagation on
those i.e. copy propagation should probably happen after matrix /
struct / array splitting.



More information about the wine-devel mailing list