- User Since
- Feb 13 2015, 12:29 AM (218 w, 5 d)
Sun, Apr 14
Mon, Apr 1
Rebase and ping.
Sat, Mar 30
Ok, abandoning it as it doesn't seems to be necessary.
Mar 22 2019
Is someone of a different opinion than @spatel ? If not, I'll abandon this patch soon.
Mar 12 2019
@spatel I'm not sure, you tell me :) The history of this patch is that this pattern shows up in practice post legalization, so I submitted a patch for it for DAGCombiner. From there, @ lebedev.ri noticed that InstCombine wasn't doing it and suggested it to be added there as well.
Mar 11 2019
- Fix types
- Remove unnecessary use of auto
- Use a lambda
Mar 10 2019
Remove dependency on D57367 as now it show improvement on existing test cases.
Mar 8 2019
Update to reflect change in tests (the vector test had an incorrect constant in it).
Rebase and regenerate tests introduced in rL355400
Mar 3 2019
All the pattern I submit in DAGCombiner are extracted from actual code -in this case libraries doing cryptography). I never encountered the vector version of this but I'm happy to add test cases for them if this is of interest.
Add extra ops to ensure proper ordering.
Mar 2 2019
I made the patch for InstCombine: D58877
It is interesting that instcombine doesn't do this. I would have expected it to be the case. Let me prepare a patch for it.
Mar 1 2019
Rebase and add test cases taht do not depend on D57367
Feb 21 2019
@niravd Any progress ?
rebase on top of rL353626
Feb 8 2019
@RKSimon This can go now, but has no effect in isolation in any of the test cases I have, mainly because DAGCombine is fairly powerless when faced with addcarry as most of the work involve 'deep' patterns. Do you think I should land anyways ?
Feb 4 2019
Jan 31 2019
Jan 29 2019
Jan 28 2019
Rebase on top of D57367
I was thinking about ways to reduce the overhead created by this change. I came up with D57367, which is an alternative that focuses on nodes likely to benefit from the change instead of the whole DAG. It misses several opportunity that exist in that patch, but it seems to be a tradeof worth doing.
First about the kind of code I try to get to have better codegen, it's mostly about large integer manipulations. I already added a fair amount of reduced test cases in addcarry.ll/subcarry.ll . I'm at a stage where the pattern I have to work with are somewhat deep, see D57302 for an example. These patterns do not do anything useful if other transform cannot pick up from whee they left.
Add a comment about existing commutative ops deduplication logic.
Jan 27 2019
Fix NewY's type which was invalid in some circustances
I get it now. We had a mismatched interpretation of which side is up, which side is down. If this is indeed the order in which nodes are processed, then it'd be beneficial to change this.
After thinking more about this, I do not think going bottom up is a good idea. All patterns match a node + its operands, and so benefit from operand to be combined themselves already. I do not think changing all the patterns to match use rather than operands is a good idea. This is a ton of work, and this is unclear there is any benefit at all.
Jan 26 2019
I'm not sure how processing nodes bottom up really helps. Problems arise when you want to use patterns of depth > 2 because then direct parent.child are not processed again, even though such pattern may now be available. It seems to me that both top/down and bottom/up approaches would suffers from the same problem, but maybe there is something I'm missing.
I'd like to ressurect this diff.
Aug 15 2018
@atanasyan The behavior is not supposed to change in any way, so test cases should not change. However, there are doubt about semantic for which there is no test case today.
Jun 6 2018
Jun 5 2018
Jun 4 2018
Jun 3 2018
Jun 1 2018
Revert changes in LanaiAluCode.h
The change in LanaiAluCode.h are indeed unrelated, let me remove them.
May 30 2018
Add a mention of the change in the release notes.