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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
