[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385
+; CHECK-NEXT:    [[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]]
+; CHECK-NEXT:    ret i1 [[OR]]
 ;
----------------
nikic wrote:
> aqjune wrote:
> > nikic wrote:
> > > It looks like this fold could be salvaged, if we wanted to: https://alive2.llvm.org/ce/z/TpsYAj
> > Thx, I added the transformation.
> > If the transformations look good, I'll make it as a separate commit with tests and push it.
> Could you please split it off into a separate review? Hard to understand impact as part of this change.
It is splitted into D101375


================
Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20
 
+; FIXME: noundef should be attached to args
 define i1 @will_not_overflow(i64 %arg, i64 %arg1) {
----------------
spatel wrote:
> Any ideas about what it will take to get those argument attributes for the C++ source example shown in the code comment?
> 
> SDAG is still going to convert the `select` to `and`, so we can probably avoid regressions by replicating InstSimplify's omitCheckForZeroBeforeMulWithOverflow() as a DAG combine. Let me know if I should do that.
I promised to do the patch at D82317, but these days I'm occupied with other things, so it might not be a recent future (not in a month at least)...

I think it is a good chance to use freeze here: We can add
```
%cond = icmp ne %x, 0
%v = call @llvm.umul.with.overflow(%x, %y)
%ov = extractvalue %v, 1
%res = select i1 %cond, %ov, false
  =>
%y.fr = freeze %y
%v = call @llvm.umul.with.overflow(%x, %y.fr)
%ov = extractvalue %v, 1
%res = %ov
```
into CodeGenPrepare.
What do you think? CodeGenPrepare is already using freeze for a few transformations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101191/new/

https://reviews.llvm.org/D101191