[PATCH] D64713: [InstCombine] X *fast (C ? 1.0 : 0.0) -> C ? X : 0.0

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/mul.ll:522-530
+define i32 @mul_div_select(i32 %x, i32 %y, i1 %c) {
+; CHECK-LABEL: @mul_div_select(
+; CHECK-NEXT:    [[MUL:%.*]] = select i1 [[C:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[MUL]]
+;
+  %div = udiv exact i32 %x, %y
+  %sel = select i1 %c, i32 %div, i32 1
----------------
foad wrote:
> spatel wrote:
> > foad wrote:
> > > spatel wrote:
> > > > Is this test changed by this patch? If not, it shouldn't be here.
> > > Yes. Without this patch, this test does not get simplified at all.
> > Ok, then please commit this test with baseline CHECK lines as another preliminary NFC step ahead of this patch.
> I don't really understand why you're suggesting to do that with this test case, but not with the other tests I added. There really is not much difference between them.
> 
> Perhaps it would be clearer if I split my original patch into two:
> 1. Pass fast math flags in to the calls to simplifyBinOp.
> 2. Add the `X op (C ? P : Q) -> C ? (X op P) : (X op Q)` combines that are really the main point of the patch.
Yes, please add all of the baseline tests for each part of this patch and fork this into the minimal code change patches, so we can see each piece independently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64713