[Bug target/43902] suboptimal MIPS widening multiply accumulate

["wilson at codesourcery dot com" <gcc-bugzilla@gcc.gnu.org>]

------- 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