------- Comment #10 from wilson at codesourcery dot com 2010-06-16 06:30 ------- Subject: Re: suboptimal MIPS widening multiply accumulate
On Wed, 2010-06-09 at 20:21 +0000, bernds at gcc dot gnu dot org wrote: > What do you think? Please let me know what your MIPS tests turned up. I'm looking at this again. My MIPS tests showed that my patch fixed 17 gcc.target/mips multiply-accumulate testcases that were accidentally broken by the new tree level widening multiply optimization pass. My new testcase madd-9.c failed, but it turned out that I accidentally double-patched the file. Fixing the file, it now passes. I forgot to include this testcase in my second patch, though it was there in the first one. The bad news is that there are two new failures: dpaq_sa_l_w.c and dpsq_sa_l_w.c. These are MIPS DSP fixed-point multiply-accumulate testcases, which is something I definitely didn't bother to check earlier. Overall I would say it is a win, since it fixes many int/long/long long examples, and only breaks a few obscure cases that should be fixable with a little more work. I'm looking at your patch too. There is a testcase that doesn't appear to belong, gcc.target/arm/pr42172-1.c. I'm not sure why convert_plusminus_to_widen needs to check for MULT and call convert_mult_to_widen. If we are scanning basic blocks from top to bottom, it seems that the multiplies would have already been checked. But maybe this has something to do with looking at operands computed in other basic blocks that haven't been scanned yet, in which case it would make sense. I'm not sure if that is possible. Otherwise, it looks like you have completed and cleanup up a bunch of stuff that I didn't get far enough to notice. It all looks good to me. I can try rerunning MIPS testcases to see how it works. I see that the failure of the DSP fixed-point tests is because we have checks for + if (TREE_CODE (type) != INTEGER_TYPE) + return false; and the old code in expr.c that we are replacing does - if ((TREE_CODE (type) == INTEGER_TYPE - || TREE_CODE (type) == FIXED_POINT_TYPE) So that looks like an easy fix (assuming no other complications), but will require a rebuild and retest. Jim -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43902
