[patch] Don't change the types for switch statements

[Jeffrey A Law]

On Thu, 2005-03-10 at 00:44 -0500, James A. Morrison wrote:
> Jeffrey A Law <law@redhat.com> writes:
> 
> > On Wed, 2005-03-09 at 23:46 -0500, James A. Morrison wrote:
> > >  Hi,
> > > 
> > >   This patch fixes the latent bug exposed by my fix to pr15784.  index_type
> > > was being unconditionally set to integer_type_node when try_casesi failed.
> > > Unfortunatly, not all the tree's being used in the switch statements were
> > > integer_type_nodes.  This fixes the problem, by not changing index_type.
> > > Not too long ago index_type was changed after try_casesi to a value stored
> > > in a case specific datastructure, but that was removed last year.  See
> > > cvs diff -up -r1.377 -r1.378 stmt.c for more details on that change.
> > > 
> > >   I'm bootstrapping this patch on ia64-linux with the patch to pr15784 and
> > > Ada.  So far the bootstrap has got passed the ice in c-pragma.c or any of
> > > the Ada code that triggered the problem.  I'm also bootstrapping this patch
> > > on sparc-linux.  Ok for mainline and regtesting if bootstrap succeeds on
> > > both ia64 (with pr15784 patch*) and sparc (without 15784) patch?
> > Another approach is to fix the type of the MINUS_EXPR so that
> > it has the same type as minval and n->{high,low}.  I actually think
> > that's safer since with your change we'll still be creating (and
> > folding) MINUS_EXPR nodes where the type of the MINUS_EXPR does not
> > match the types of both operands.
> 
>  You'll have to fix the types in more than this one place.  It seems to be
> assumed that index_type is the same type of n->{low,high} and minval/maxval
> in expand_case.
I see 3-4 places in expand_case where we potentially make this
assumption.  I don't think it would be terribly hard to fix them
(famous last words).


>   However, when index_type is reassigned to integer_type_node,
> this assumption is no longer true,
Are you sure that if we don't muck up index_type that it will have
the same types as the nodes themselves.  I vaguely recalling this
changing a while back.


>  so I think the problem is reassigning
> index_type, not the call to build.  Right now we create nodes that don't
> match, but with my patch the type of the nodes does match.
Possibly.  Ultimately I think it comes down to determining if the
original index_type is the same as the types of the nodes in the
case list.  If it is, then I'd tend to lean towards your approach rather
than mine.

> 
>  On a similar note, there isn't any reason to use fold (build (MINUS_EXPR ..))
> as int_const_binop (MINUS_EXPR ...) would do the right thing here.  That
> only works around the problem of index_type being the wrong type sometimes
> though.
Right.  I think that change would stand on its own since ultimately it
results in the compiler allocating less memory and doing less work.
 
jeff