On Fri Jan 13 00:08:40 2023 +0000, Zebediah Figura wrote:
A couple non-obvious notes regarding the changes I made to your patches—
- I added a comment explaining why we have these copy-propagation
passes. Especially because even now, every time I look at the pass I have to look up and remember why we can't just vectorize *all* stores.
- I got rid of the type check from
copy_propagation_compute_constant_replacement(). We already filter out complex types in the caller, and there's not much point preventing object types from showing up here since they can't have constant values anyway.
I didn't thought of making `copy_propagation_compute_replacement()` and `copy_propagation_compute_load_constant_replacement()` replace the instruction themselves while calling `copy_propagation_compute_replacement()` directly from `copy_propagation_transform_object_load()`, that makes the code more clear and makes us avoid the output swizzle argument. So I think it is a very good arrangement of the solution.
Also, I think the included comment is a good summary of all this conundrum! I would rewrote the pseudo code examples to make them more clear, using one line for each instruction: ``` X is stored to V There is a load from V ``` ``` X (constant) is stored to a part of V Y (constant) is stored to another part of V There is a load from V ```
Also, we don't have to forget including the "Fold swizzle chains" commit 72e581872e2320b928dbb5d13f57c41548e24a21!