@RKSimon It is good to go, the main problem here is that I don't have a test case for it in isolation, and I have been struggling to get the pieces in where this really shines (mostly related to process the DAG topologically).

This is lacking a test. The LLVM C API is typically tested using the echo test (a program that read LLVM IR with the LLVM C API, and the re-emit it, and it can be checked that both are equal).

Do you need help merging this?

I will land this for you, no problem.

Ok, this looks like it is good to go then. @RKSimon , if this is good for you, then we shoudl land this.

Fix aix32-cc-abi-vaarg.ll

Rebase on top of D124756 .

Keep the unoptimized test case.

Update for the monorepo

Hi @aykevl . I unfortunately am not familiar with thin LTO, so it is very difficult for me to review this. Maybe @mehdi_amini could help? I know he worked on this at some point, and if he cannot, he'll know who can.

Thanks.

Wouldn't LLVMBuildBinOp build a constant if both operand are constant and the operation allows for it? I have not double checked, but I think it does. Considering this, I'm not really sure what this brings to the table.

I think using nullptr would be preferable, but this looks good. Thanks.

This is something that I wanted to do for a long but never had time to. Thanks.

It would be good to have a test case for this.

Ping, rebase

That is indeed the step I was missing. Thanks everybody for getting me up to speed.

In D68195#1757817, @lebedev.ri wrote:

In D68195#1757811, @deadalnix wrote:

Alright, this is the first time I have to commit something with the new github process, and I have to admit, I do not know how to do it - or if I can do it at all. Running git llvm push ask me for a username and password, and my old one do not work. What's the next step for me here?

Did you ever pushed anything to github before, to any other repository?
Did you clone https://github.com/llvm/llvm-project/ via HTTPS or SSH? (should be former!)
It is likely asking for your github login and password.

You may *really* want to follow https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line to create credentials with which you can *only* commit, but do nothing else

Alright, this is the first time I have to commit something with the new github process, and I have to admit, I do not know how to do it - or if I can do it at all. Running git llvm push ask me for a username and password, and my old one do not work. What's the next step for me here?

Address some of the comments

I'm a bit late to the party, but this is nothing short of amazing. Congratulations!

Also while I'm at it, first sorry for taking so long to review, I missed that patch. Second, if I understand properly, getting node to be processed in the right order actually can collapse arbitrary long addition chains, but it's not clear how to get there, there is a bit of a wack a mole problem that start poping up every time.

Can you please explain a bit more what this is doing? This seems interesting, but I'm not quite sure the pattern is correct.

O => Op

It's indeed not clear if what you have is the result of the legalizer. In this case just proceed.

Add a few comments

Rebase and ping.

Fix erroneously inverted condition and ensuring previously broken tests are now passing again.

Add test for vector - but nothing changes for them (see rL374043 ) and use isSubsetOf and getAllOnesValue.

Use isConstOrConstSplat and getAPIntValue

Rebase on top of NFC changes

Ping ?

Add tests for the commutative case.
Precommit tests.

Tighten the checks before creating the node.

• Avoid creating the shift node when not stricly required.
• Add negative tests.

I looked at the problem that @spatel ran into. It is not applicable to DAGCombiner, because the DAG is processed only once rather than in a loop as long as it is modified. I wouldn't be possible to change it to work like InstCombine does as there are a ton of A -> B -> A type of tranforms. That being said, it's not a good reason to add more, so it's worth looking into improving this if possible.

In D66882#1649078, @lebedev.ri wrote:

Pattern look good to me, but i recall having disscussion in https://reviews.llvm.org/D64275#inline-576684
Granted, that is InstCombine, but what happens here if we create instruction but then fail to fold?

IndexC => ExtrIndex

Upadte the doc comment to make it clear that the Mask argument can be modified.

@nathanchance Thanks for this. Having a test case will help tremendously. Sorry for the inconvenience.

Rebase on top of D66804