[PATCH] D70237: [X86] Add more addcarry tests

David Zarzycki via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 14 06:54:15 PST 2019


davezarzycki added a reviewer: craig.topper.
davezarzycki added a subscriber: craig.topper.
davezarzycki added a comment.

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.
3. What is add256_1 trying to do and how is it different than add256_2?
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.


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