[PATCH] D48223: DAG combine "and|or (select c, -1, 0), x" -> "select c, x, 0|-1"

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 16 10:17:10 PDT 2018


rampitec added a comment.

In https://reviews.llvm.org/D48223#1134505, @xbolva00 wrote:

> What about
>  (X / Y) != 0 -> X >= Y ?
>
> (X, Y are unsigned)


This might be a good idea, but seems a separate change.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1883-1889
+  unsigned SelOpNo = 0;
   SDValue Sel = BO->getOperand(0);
+  if (Sel.getOpcode() != ISD::SELECT || !Sel.hasOneUse()) {
+    SelOpNo = 1;
+    Sel = BO->getOperand(1);
+  }
+
----------------
spatel wrote:
> This change is independent of the and/or enhancement. We're now allowing folding when the binop has a constant operand 0. That seems like a good enhancement for non-commutative binops, but it should have its own tests using opcodes besides 'and'/'or' and be split into its own patch.
> 
> Example based on one of the original tests from rL296699:
> 
> ```
> define <2 x double> @sel_constants_fmul_constant_vec(i1 %cond) {
>   %sel = select i1 %cond, <2 x double> <double -4.0, double 12.0>, <2 x double> <double 23.3, double 11.0>
>   %bo = fsub <2 x double> <double 5.1, double 3.14>, %sel
>   ret <2 x double> %bo
> }
> 
> ```
I am not sure why this would be beneficial. For example:

```
sub (select 0, -1), x -> select (sub 0, x), (sub -1, x)
add (select 0, -1), x -> select x, (add -1, x)

```
In this case select would remain and two subs instead of one, seems worse.
For add both select and add would remain, so the benefit is not obvious.

In turn and/or have clear benefit in this change because binop completely goes away, and 
In fact the same can be done to xor, but I have decided not to since xor would be needed anyway with a non-zero operand (and one would never have a select with both sizes zero).


https://reviews.llvm.org/D48223





More information about the llvm-commits mailing list