On 9/17/20 8:32 AM, Jason Merrill wrote:
We discussed this in a team meeting the other day, and agreed that it's probably simpler to switch back to gotos for C++ than fix up all the optimizers. And that there probably isn't much benefit to the middle-end to retain the higher level representation longer.
When I looked into the history of the C++ change to use LOOP_EXPR, it seemed the primary motivation was to allow the constexpr evaluator to recognize loops and the optimization benefits were only lightly tested. But now the constexpr evaluator doesn't need LOOP_EXPR, and meanwhile I think the loop infrastructure and loop optimization test cases have been tuned for the C-style output.
Though the patch needed to the Fortran compiler sounds like a regression from this change. Has someone looked at where that's coming from?
I tried, but wasn't able to come up with a self-contained test case small enough that I could actually analyze what it was doing, but that still triggered the warning. In fact I am still unsure whether it is a bug that we weren't diagnosing that warning before, rather than that it showed up now. :-S As a programmer looking at that code, I thought the warning was justifiable, at least.
Have you done any benchmark comparison for C++ tests with this change? If not, please plan to monitor the lnt.opensuse.org benchmark results for impact after the change is committed.
I haven't done any benchmarking myself, but yes, I will keep an eye on those benchmark results.
The C++ changes are OK. A C maintainer will need to sign off on the changes there.
Thanks for the review! -Sandra
