[PATCH] D65658: [InstCombine] Propagate fast math flags through selects

Wed Aug 7 04:25:43 PDT 2019

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

In D65658#1618618 <https://reviews.llvm.org/D65658#1618618>, @foad wrote:

> In D65658#1616800 <https://reviews.llvm.org/D65658#1616800>, @spatel wrote:
>
> > The code change seems fine, but the lone test does not provide enough coverage. Please have a look and rebase:
> >  rL368028 <https://reviews.llvm.org/rL368028>
>
>
> Done. Thanks for the test cases! Note that the fdiv case is not affected because `InstCombiner::visitFDiv` does not call `SimplifySelectsFeedingBinaryOp`. Perhaps it should.

Yes - I noticed that test wasn't changing, but I didn't track down why. If you want to mark that test with a TODO comment, that would be good. Or you could fix the fdiv logic first, then update this patch again to show the improvement for fdiv.

LGTM

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65658