[PATCH] D50222: [CodeGen] [SelectionDAG] More efficient code for X % C == 0

[Dmytro Shynkevych via Phabricator via llvm-commits]

hermord updated this revision to Diff 161419.
hermord added a comment.

Thank you for the review, @lebedev.ri. Sorry for the delay; resolved most of the issues. Quick summary of the changes:

1. BuildREMEqFold is now invoked from SimplifySetCC, as it logically should be. The signature is changed to mirror the other SimplifySetCC helpers.
2. Tests are moved to a separate file. The format is slightly changed to get rid of irrelevant `select`s. Size optimization tests are added. The Hexagon test fix is split into a separate patch: https://reviews.llvm.org/D50944.
3. The | LHS | < | RHS | optimization (call it (*)) is removed from this patch.

Note: since changes were split into two patches, this now fails tests until https://reviews.llvm.org/D50944 gets merged. Even when that happens, this will still fail `test1` in `CodeGen/X86/jump_sign` because of (*). This failure should probably be fixed by implementing the (*) in a separate patch, but I'm unsure as to the best way to go about it: since BuilREMEqFold moved out of visitREM, it will get invoked before any optimizations in visitREM have had a chance to run, as the SETCC node is processed first. As I see it, there are the following options:

1. Move BuildREMEqFold back into visitREM. This is ugly for other reasons, as @lebedev.ri pointed out, but it does solve this particular hurdle. (*) can then be added right before it.
2. Make (*) a separate function and invoke it in both visitREM and SimplifySetCC (before calling BuildREMEqFold). This may result in it running twice, which is probably bad. Computing KnownBits looks like it's expensive.
3. Check if (*) is possible in SimplifySetCC and, if so, don't touch the node. visitREM then implements the actual optimization.
4. Ignore (*) and implement an equivalent optimization for multiplication on the output of BuildREMEqFold.
5. Accept the probably rare regression (dividend must be known to be smaller than the divisor AND the result has to be fed into a comparison with zero) and edit the test.

I lack experience to make the right call here and would much appreciate any suggestions.


Repository:
  rL LLVM

https://reviews.llvm.org/D50222

Files:
  include/llvm/CodeGen/TargetLowering.h
  lib/CodeGen/SelectionDAG/TargetLowering.cpp
  test/CodeGen/X86/jump_sign.ll
  test/CodeGen/X86/rem-seteq-optsize.ll
  test/CodeGen/X86/rem-seteq.ll
  test/CodeGen/X86/vselect-avx.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50222.161419.patch
Type: text/x-patch
Size: 18657 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180820/046d10ce/attachment.bin>