- User Since
- Feb 13 2015, 12:29 AM (235 w, 4 d)
Sat, Aug 17
Fri, Aug 16
Sun, Aug 11
Rebase and ping
Jul 20 2019
Revert the addition of an unecessary empty line.
Jul 18 2019
@lebedev.ri I think these are reasonable assumptions. Just add a test case for X86 and this is good to go as far as I'm concerned.
Jul 16 2019
Maybe it is worth adding some platform dependent check to actually make sure turning the carry into a scalar is expensive? Or is it a reasonable assumption to make that it expensive on all plateforms?
Rebase now that D59208 landed on master.
Jul 11 2019
tomatch => to match
Remove default value for extractBooleanFlip's Force parameter.
Jul 3 2019
Rebased on top of D64174 , which expose many instances of this pattern.
Add a comment explainign what extractBooleanFlip does.
Rebase, extract the NFC changes into a NFC patch and remove redundant getNode call.
Jun 26 2019
rebase and ping!
Jun 12 2019
Update logic as requested
Do you have a test case where diamond carries are generated for subs ? I'm happy to look into this, but I have no test case.
Jun 2 2019
May 17 2019
This is important to be able to make the carry propagation path canonical to be able to optimise large ineteger computation further. I would really appreciate if tis could move forward as this is gating a lot of work.
May 4 2019
Rebase and ping.
Apr 14 2019
Apr 1 2019
Rebase and ping.
Mar 30 2019
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.