Missed transform: umin/umax/smin/smax on i1 to and/or

[Kmeakin]

umin(i1, i1) => and: https://alive2.llvm.org/ce/z/tRp7VK
umax(i1, i1) => or: https://alive2.llvm.org/ce/z/7nS6Wp

smin(i1, i1) => or: https://alive2.llvm.org/ce/z/XXfBSk
smax(i1, i1) => and: https://alive2.llvm.org/ce/z/N9fqT2

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Assign the issue to you.
2. Fix the issue locally.
3. Run the test suite locally.
   3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can
   e.g. use make check-clang-ast to only run Clang's AST tests.
4. Create a git commit
5. Run git clang-format HEAD~1 to format your changes.
6. Submit the patch to Phabricator.
   6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue

[elhewaty]

Hi there,
I am a total beginner in LLVM although I have a solid knowledge of compilers, and I contributed to GCC, and SpiderMonkey before.
I really want to solve this, but can anyone please give me a hint about where to start?
Where in the source should I find this? or any information that can help?

[RKSimon]

I'd expect it'd probably get added inside InstCombinerImpl::visitCallInst for the smin/smax/umin/umax case:

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

Line 1599 in 479955a

case Intrinsic::smin: {

A example of the fold for add (i1, i1) -> xor (i1, i1) can be found here:

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

Line 1426 in 479955a

return BinaryOperator::CreateXor(LHS, RHS);

[elhewaty]

@RKSimon I think I fixed it, how can I test the changes?
how can I know where make check-llvm fails exactly?

[RKSimon]

Try make check-llvm-transforms - but its likely you will need to add additional tests inside llvm-project/llvm/test/Transforms/InstCombine/

[RKSimon]

@elhewaty Any luck with adding test coverage?

[overlorde]

hi @elhewaty any update of the patch?

[Qi-Hu]

Hi @elhewaty, are you still working on this issue? If you don't mind, I'd like to take over. I wrote this patch as a practice: https://reviews.llvm.org/D158915.

[Qi-Hu]

The patch has been landed: 1a65cd3fcf58. This issue can be closed.