This patch tries to combine pattern add(index_vector(zero, step), dup(X)) into index_vector(X, step)
TestPlan: check-llvm
Differential D100107
[AArch64][SVE] Combine add and index_vector junparser on Apr 8 2021, 7:13 AM. Authored by
Details This patch tries to combine pattern add(index_vector(zero, step), dup(X)) into index_vector(X, step) TestPlan: check-llvm
Diff Detail
Event TimelineThis comment was removed by junparser.
Comment Actions What I want know something more is what is the boundary between tablegen pattern match and dag combine? I never figure this out. Just use this case as example, we can implement the feature in both place, but I don't know how to handle commutivity in tablegen (maybe SDNPCommutative? I don't know.) , and their effects on the next step: combine load/store with index_vector.
Comment Actions There isn't a firm rule to follow re tablegen pattern vs manual dag combine, but I think the steer is that when the combine is needed after legalization and it's possible to specify as a pattern, a pattern is preferred. In this case, probably a pattern would be a clean way to implement it. I haven't used SDNPCommutative before, but that looks like it should do the trick.
Comment Actions Thanks for explain this.
Comment Actions
Comment Actions Thanks, that looks quite neat. Can you also add a few tests when there is >1 use of the stepvector (e.g. using the stepvector in two adds), so we can test the fold indeed doesn't happen?
Comment Actions Other than my comment on two missing tests, the patch looks good to me.
|
Hi @junparser, there are a few things to take into account here:
This can probably be implemented quite easily in TableGen with some patterns that try to do the fold, but where the PatFrag itself has a condition that it only has a single use that this use is not the address in a MemSDNode. See e.g. AArch64mul_p_oneuse where it also checks for a single use before selecting an MLA/MLS.
Because of how we have currently implemented the legalization of gathers/scatters, I don't think you can currently test the condition that the node is not used in a MemSDNode, but I'm not sure if that matters much.