[PATCH] D48223: Allow binop C1, (select cc, CF, CT) -> select folding

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1913-1914
   EVT VT = Sel.getValueType();
+  if (SelOpNo && VT != C1.getValueType())
+    return SDValue();
+
----------------
spatel wrote:
> When does this happen (is there a test)? Why does this only happen when the select is operand 1 of the binop? Better to remove the 'SelOpNo' condition to be safer?
That is if shift value and amount have different types. On x86 shift amount is i8 regardless of LHS. The new test shl_constant_sel_constants does not fold on x86 but does on amdgpu. W/o the check this test asserts. At the same time old test does work on x86 and folds correctly, so I think it is only needed if I have swapped operands. In general that should be possible to write a piece of code to create correct VT for shifts, but I'd better leave that to x86 folks. I guess range checking will be also needed if such a code is about to be written.


https://reviews.llvm.org/D48223