[PATCH] D72675: Fix -ffast-math/-ffp-contract interaction

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D72675#1824659 <https://reviews.llvm.org/D72675#1824659>, @wristow wrote:

> This all sounds good to me.
>
> So to make sure we're all on the same page, my understanding is that the plan forward is:
>
> 1. Make the Clang change first (including adding another pair of tests for `-ffp-contract=on`).
> 2. Update the LLVM tests illustrating the current baseline state.
> 3. Make the LLVM change shown here, and update the tests with the fixes.
> 4. Bring in the DAG combiner work that Michael has done internally at Apple.
>
>   Points 1, 2, and 3 are essentially the points in the patch posted here, so I'll do that.  And of course Michael will then take on point 4.
>
>   Is that the plan?  If yes, I'll transition this item to just be the Clang pieces, and I'll spin off a new one to do the LLVM portion of what is posted here.


SGTM

> Sanjay, regarding:
> 
>> But it would be better to have all of the baseline tests in place, so we know where we stand currently.
> 
> I'm interpreting that as applying to the LLVM tests.  That is, the Clang change, along with the updated tests, can be in one commit, rather than first updating the Clang driver tests with the current state.  If you'd prefer two commits on the Clang side as well, let me know.

One commit for the clang changes should be ok; it's a very small diff. But I'm still not sure if the driver change induces frontend diffs that we should make visible via tests.


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

https://reviews.llvm.org/D72675