[PATCH] D52504: [DAGCombiner] Div/rem folds

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3103
-  // TODO: X % 1 -> 0
-  // If this is a boolean op (single-bit element type), we can't have
-  // division-by-zero or remainder-by-zero, so assume the divisor is 1.
----------------
spatel wrote:
> Please don't remove this comment without actually fixing the underlying problem (in a separate patch).
> I think it would be better to consolidate the existing code here for 2 reasons:
> 1. It's cleaner to have all of the related folds in 1 place.
> 2. The existing simplification for urem/srem-by-1 is not particularly efficient - it converts to an 'and' first and then simplifies the 'and'.
> 
> Here are test cases you can add/use to confirm the difference between llc and opt for the boolean patterns:
> 
> ```
> define i1 @bool_udiv(i1 %x, i1 %y) {
>   %r = udiv i1 %x, %y
>   ret i1 %r
> }
> 
> define i1 @bool_sdiv(i1 %x, i1 %y) {
>   %r = sdiv i1 %x, %y
>   ret i1 %r
> }
> 
> define i1 @bool_urem(i1 %x, i1 %y) {
>   %r = urem i1 %x, %y
>   ret i1 %r
> }
> 
> define i1 @bool_srem(i1 %x, i1 %y) {
>   %r = srem i1 %x, %y
>   ret i1 %r
> }
> 
> define <4 x i1> @boolvec_udiv(<4 x i1> %x, <4 x i1> %y) {
>   %r = udiv <4 x i1> %x, %y
>   ret <4 x i1> %r
> }
> 
> define <4 x i1> @boolvec_sdiv(<4 x i1> %x, <4 x i1> %y) {
>   %r = sdiv <4 x i1> %x, %y
>   ret <4 x i1> %r
> }
> 
> define <4 x i1> @boolvec_urem(<4 x i1> %x, <4 x i1> %y) {
>   %r = urem <4 x i1> %x, %y
>   ret <4 x i1> %r
> }
> 
> define <4 x i1> @boolvec_srem(<4 x i1> %x, <4 x i1> %y) {
>   %r = srem <4 x i1> %x, %y
>   ret <4 x i1> %r
> }
> 
> ```
I will revert that comment removal..

Anyway, I see   

// fold (sdiv X, 1) -> X
  if (N1C && N1C->isOne())
    return N0;

in visitS(U)DIV so.. do you suggest to unify it and move it here?


https://reviews.llvm.org/D52504