On 9/9/20 8:20 PM, Sandra Loosemore wrote:
On 9/9/20 3:13 PM, Jason Merrill wrote:

My impression from Jeff's analysis in January and David's in March was that many of the testsuite changes were from the C++ approach actually providing better results, so the reversal here surprises me.  Can you talk more about the regressions you're seeing?

I spent most of my time earlier in the summer looking at the 3 regressions I originally saw last fall.  Unfortunately I could get no traction at all on the 2 FSA-related ones.  :-(  For the gcc.dg/tree-ssa/ssa-dce-3.c regression, I tracked it down to some code in the cddce1 pass being sensitive to the ordering of basic blocks in the input code;  I filed PR96487 for that.

I was also having a bunch of problems with -Wimplicit-fallthrough failures triggered by the C++ front end loop expansion sometimes flipping the sense of the end test conditional (through the use of fold_build3_loc to build it).  It seemed to me that the code that handles COND_EXPRs for those warnings is sensitive to the order of blocks as well, and has asymmetric assumptions that the code for the "if" branch is emitted inline before the "else" branch.  I tried some experiments with generalizing it to recognize the branches in either order, but that did not fix the regressions, so maybe the problem was somewhere else entirely, or a combination of two different bugs.  :-(

Anyway it seemed to me that the patches would not be accepted if I resubmitted them in a form that still caused regressions, and switching back to the C way of expanding to GOTO form directly instead of via LOOP_EXPR did that.

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.

Though the patch needed to the Fortran compiler sounds like a regression from this change. Has someone looked at where that's coming from?

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.

The C++ changes are OK. A C maintainer will need to sign off on the changes there.

Jason

Reply via email to