diff mbox

Extend widening_mul pass to handle fixed-point types

Message ID 1280249995.18689.48.camel@e102325-lin.cambridge.arm.com
State New
Headers show

Commit Message

Ramana Radhakrishnan July 27, 2010, 4:59 p.m. UTC
On Tue, 2010-07-27 at 17:51 +0200, Bernd Schmidt wrote:
> On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote:
> > 
> > On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote:
> >> That's pretty much what I had in mind.  Ok if it passes testing.
> > 
> > This broke bootstraps on arm-linux-gnueabi and caused
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . 
> > 
> > Taking a brief look at it this morning, I think the problem here is that
> > the call to optab_for_tree_code is made with the wrong type of the
> > expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR
> > should be based on the type of ops->op2 rather than opnd0 because you
> > have something like intval0 = shortval1 * shortval2 + intval1 whereas
> > the expander is testing for a widening mult and plus where the results
> > are into a HImode value and thus effectively for a widening operation
> > into an HImode value.
> > 
> > The assert that fails is because 
> > gcc_assert (icode != CODE_FOR_nothing);
> > 
> > Something like this fixes the problem for the test-cases under question
> > or would you prefer something else. A full bootstrap and test is
> > underway.
> > 
> > Ok to commit if no regressions ? 
> 
> I think this is consistent with the code in tree-ssa-math-opts (there we
> use the type of the lhs, which should match that of operand 2).  So, OK
> - I think the ARM testsuite has some widening-macc cases so you should
> notice if there's something wrong.

Yep so I thought as well. Yes the tests in gcc.target/arm should cover
these cases.

Bootstrap proceeded further and ICE'd again in stage2 because we were
applying gimple_assign_lhs on a non GIMPLE_ASSIGN statement. 

Attached is a simple test case that shows this problem with a
cross-compiler as well. 

I am testing with this extra patch[1] applied on top of my previous
patch which I think is obvious and will see what else is needed to be
done to deal with the fallout of this patch with bootstrap on
arm-linux-gnueabi. Verified with a simple testcase that we still do
generate widening multiplies for the original testcase. I'll submit both
these after I'm sure that bootstrap and regression testing completes. 


cheers
Ramana



1. New patch being tested on top.

2010-07-27  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	* tree-ssa-math-opts.c (is_widening_mult_p): Return false 
	if not an assignment.

[ramrad01@e102325-lin gcc]$ svndiff tree-ssa-math-opts.c
diff mbox

Patch

Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c        (revision 162571)
+++ tree-ssa-math-opts.c        (working copy)
@@ -1323,6 +1323,9 @@  is_widening_mult_p (gimple stmt,
 {
   tree type;
 
+  if (!is_gimple_assign (stmt))
+    return false;
+
   type = TREE_TYPE (gimple_assign_lhs (stmt));
   if (TREE_CODE (type) != INTEGER_TYPE
       && TREE_CODE (type) != FIXED_POINT_TYPE)