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

Wed Jul 17 23:07:05 PDT 2019

foad marked an inline comment as done.
foad added inline comments.

================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:778-779
+    Cond = A;
+    True = SimplifyFPBinOp(Opcode, B, E, FMF, Q);
+    False = SimplifyFPBinOp(Opcode, C, F, FMF, Q);
+
----------------
spatel wrote:
> foad wrote:
> > spatel wrote:
> > > I know this is an existing function name, but it doesn't match my expectation based on the name. Change it to simplifyAnyBinOp() or simplifyIntOrFPBinOp() as a preliminary step?
> > I don't think your suggestions are any better than the status quo; how is anyone supposed to know or remember the difference between `simplifyBinOp` and `simplifyAnyBinOp`? Instead, how about I overload `simplifyBinOp` to take FMF as an optional (last) argument?
> Ok, that's less misleading than what we have now. You can make that change without review, then rebase this patch.
See D64902. I know you said I could do it without review, but I prefer to give others a chance to comment before committing.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64713