Hey, I noticed that this does not always work for subtraction, probably due to canonicalization. See bug report: https://bugs.llvm.org/show_bug.cgi?id=49225.

This check can be useful in other case like this:

Thanks for comments @deadalnix, but please review D70079. I'm closing this one as being the duplicate of the mentioned PR.

Fix typo

Thanks for the update. Where is the original source for sub_U256_without_i128_or_recursive? The IR is strange. It seems to use two different strategies for merging carry flags.

Remove URLs

I ended up with optimizing my bigint library. The end result is - add_U256_without_i128_or_recursive which works better in current LLVM release and is also nicely reduced by D70079.
The analogous implementations of subtraction is added as sub_U256_without_i128_or_recursive test. This one should be interested to you @davezarzycki, your changes make it better, but it looks it is not perfect yet.

Update addcarry tests and add a subcarry test

In D70237#1747220, @davezarzycki wrote:

Yes, please update subcarry.ll too.

In D70237#1745687, @davezarzycki wrote:

A few comments / questions:

1. I'm not a regular contributor to LLVM, so please wait for somebody else (like @craig.topper, @RKSimon, or @spatel) to sign off on this.
2. Once one has "add carry", one immediately wants "sub carry/borrow". Please consider adding tests to subcarry.ll.

Add comments more descriptive test names

Oh sh*i, this is the same thing I'm doing in https://reviews.llvm.org/D70012.

Another round of changes.

Update unit tests.

Hi again,

I did a small update.

Update with some intermidiate changes.

In D56214#1344055, @lebedev.ri wrote:

Also, i don't think this should be hardcoded to some particular bitwidth.

Looks good, but adding an unit test for this case would be nice...

Will not that penalize stack usage on 64-bit targets as well?

Does this affects also bigger types like i256?

Fixed in https://reviews.llvm.org/rL286998.

Ping.

The algorithm is not correct. I fixed it in https://reviews.llvm.org/D26628.

This produces incorrect results for my set of tests of mul i256. Can you explain how you tested it? I guess there is no chance for adding runtime tests for that?

Add nounwind attribute to make tests smaller

Update tests

As you are doing something with YAML, can you take a look here?

As you are doing something with YAML, can you take a look here?

I tried something similar in https://reviews.llvm.org/D8770

Ping².

Ping².

In D23965#527297, @zturner wrote:

In D23965#527296, @majnemer wrote:

Sounds like a useful API. The term I've heard of in other programing languages is "take", I think take_front and take_back might be a little nicer.

take_front sounds ok, but take_back sounds awkward to me. I almost want to say "take what back?" If the convention was drop and rdrop, then take and rtake would sound good, but I'm not crazy about take_front and take_back. I could probably be convinced though if the majority prefers it.

Ping.

Please see the alternative https://reviews.llvm.org/D22619.

Ok, thanks. Any thoughts about the alternative "solution"?

Anyway, I think is it good point to at least separate the changes as you suggested.

Leave defines config as it was before.

In D21553#463304, @jyknight wrote:

Not requiring the defines is good; removing the defines in the build files is not. We still want to be able to build out of the box on systems with this known brokenness.

Is there a way to test this commit on buildbot before merging it?

This has been fixed in rL264352.

Thanks for adding that. Does it handle only 256-bit integers?

Added authors/reviewers of original patch adding home_directory() function in http://reviews.llvm.org/D2199

In D14446#296637, @rafael wrote:

E.g. in llvm-lit testing environment.

Isn't the intention precisely to avoid tests creating files in the
home directory or being dependent on global config entries?

Cheers,
Rafael