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