Re: Extend widening_mul pass to handle fixed-point types

[Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>]

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


/* ./cc1 -O2 -mcpu=cortex-a9 -quiet reduced.c   */
typedef union tree_node *tree;
enum tree_code {
ERROR_MARK,
};
enum tree_code_class {
  tcc_type,
};
extern const enum tree_code_class tree_code_type[];
struct tree_base {
  __extension__ enum tree_code code : 16;
  unsigned unsigned_flag : 1;
};
struct tree_type {
  unsigned int precision : 10;
};
union
                                                         tree_node {
  struct tree_base base;
  struct tree_type type;
};
enum integer_type_kind
{
  itk_int,
  itk_none
};
extern tree integer_types[itk_none];
typedef struct cpp_reader cpp_reader;
enum c_tree_index
{
    CTI_INTMAX_TYPE,
    CTI_MAX
};
extern tree c_global_trees[CTI_MAX];
static void builtin_define_constants (const char *, tree);
static void
builtin_define_stdint_macros (void)
{
  builtin_define_constants ("__INTMAX_C", c_global_trees[CTI_INTMAX_TYPE]);
}
void
c_cpp_builtins (cpp_reader *pfile)
{
  builtin_define_stdint_macros ();
}
static const char *
type_suffix (tree type)
{
  static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL" };
  int unsigned_suffix;
  int is_long;
  unsigned_suffix = (__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1084, __FUNCTION__); __t; })->base.unsigned_flag);
  if ((__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision) < (__extension__ ({ __typeof (integer_types[itk_int]) const __t = (integer_types[itk_int]); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision))
  return suffixes[is_long * 2 + unsigned_suffix];
}
static void
builtin_define_constants (const char *macro, tree type)
{
  const char *suffix;
  char *buf;
  suffix = type_suffix (type);
  if (suffix[0] == 0)
    {
      buf = (char *) __builtin_alloca(strlen (macro) + 6);
    }
}