@niravd Any progress ?

rebase on top of rL353626

@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 ?

Ping ?

@RKSimon That sound reasonable. The main motivation for this patch is to fix a regression introduced by D57367 , so would that be possible to get at least a concept ack on it ?

Fix typos.
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.

In D33587#1374792, @craig.topper wrote:

I don't see changes to addcarry.ll and subborrow.ll in this patch. So do we not have test cases from your workloads that show the benefit of this patch?

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.

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.

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.

@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.

Ping ?

Absolutely.

Ping ?

Revert changes in LanaiAluCode.h

The change in LanaiAluCode.h are indeed unrelated, let me remove them.

LGTM.

Add a mention of the change in the release notes.

That being said, you are correct, I should mention this in the release note and warn on the ML.

There are numerous problem with ADDC/ADDE and the sub equivalent, specifically because they are glues :) In addition, it doesn't really simplify much because there is still UADDO and friend to support, regardless.

Use isBitwiseNot
Merge flipBooleanConstant into flipBoolean
Add more specific test cases.

Add bugfix for Hexagon and rebase

Add a comment to explain the check for MVT::i16 .

@niravd I added a test case in rL323868 . The problem was by trying to extract the lower 32 bits of a 16 bit register, which is obviously not possible.

Reduce the diff to simply avoid doing the high register trick.
It turns out that there is a bug in the other optimization made in the orginal diff, so it seems like a better idea to split that out in several smaller steps to ensure progress.

Ok sounds like this isn't the right approach, closing this one.

Remove leftover DEBUG