[PATCH] D70237: [X86] Add more addcarry tests
Paweł Bylica via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 15 01:15:57 PST 2019
chfast added a comment.
In D70237#1745687 <https://reviews.llvm.org/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.
Yes, I can prepare similar tests for subtraction. Let me know if they should be include in this changeset.
> 3. What is add256_1 trying to do and how is it different than add256_2?
They are now described in comments (also the order has been swapped). Let me know if the descriptions are good enough.
> 4. Finally, can you rename your examples to be more descriptive? In particular, if you look at early parts of the file, you'll see that there are a bunch of definitions with simple names like "add256" where 256 (etc) refers to native LLVM types, not aggregates. Perhaps names like the tests I added "sub_U320_without_i128_or" which strongly implies at the implementation strategy.
Done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70237/new/
https://reviews.llvm.org/D70237
More information about the llvm-commits
mailing list