This patch started out to be a purely mechanical change to the switch 
statements so that the ones that are used to take apart constants can be 
logically grouped.    This is important for the next patch that I will 
submit this week that frees the rtl level from only being able to 
represent large integer constants with two HWIs.

I sent the patch to Richard Sandiford and when the comments came back 
from him, this patch turned into something that actually has real 
semantic changes.   (His comments are enclosed below.)   I did almost 
all of Richard's changes because he is generally right about such 
things, but it does mean that the patch has to be more carefully 
reviewed.   Richard does not count his comments as a review.

The patch has, of course, been properly tested on x86-64.

Any comments?  Ok for commit?

Kenny

Richard's comments:
=========================
The omission of CONST_FIXED from the cselib_expand_value_rtx_1,
attr_copy_rtx, clear_struct_flag and combine switches looks
unintentional (though only as a missed compiler-speed optimisation).
Same goes for the omission of CONST_VECTOR from check_maybe_invariant.

The omission of CONST_FIXED from dse.c:const_or_frame_p looks like
a missed target-code optimisation.  The function ought to be using
CONSTANT_P instead.

==== I did not do what is suggested in the last sentence because
==== it changes the behavior of the rtx "HIGH".

I don't see any reason to prefer the hashing of CONST_VECTORs in
hash_invariant_expr_1 and invariant_expr_equal_p over the default
cse implementation, so here too I think we can add CONST_VECTOR
to the switch.

cse.c:exp_equiv_p, rtx_equal_for_memref_p, operands_match_p,
rtx_equal_p_cb and rtx_equal_p  are all testing the property
"is pointer equality sufficient for this kind of constant".
rtx_renumbered_equal_p is too, so I think the omission of
CONST_FIXED there is again unintentional.

That leaves mark_jump_label_1.  This block is really testing
"does this rtx have no operands that might be labels",
which is again true for CONST_FIXED.  TBH, this smacks
of premature optimisation to me.  How do we know that
checking each code here is cheaper than falling through?
I doubt the switch is populated enough to merit a jump table,
and the number of executed branches might well be higher
like this when you take other codes into account.

So in terms of setting the abstraction level, I think there
are two options:

1) Define:

/* Match CONST_*s that can represent compile-time constant integers.  */
#define CASE_CONST_SCALAR_INT \
   case CONST_INT: \
   case CONST_DOUBLE

/* Match CONST_*s for which pointer equality corresponds to value 
equality.  */
#define CASE_CONST_UNIQUE \
   case CONST_INT: \
   case CONST_DOUBLE: \
   case CONST_FIXED

/* Match all CONST_* rtxes.  */
#define CASE_CONST_ANY \
   case CONST_INT: \
   case CONST_DOUBLE: \
   case CONST_FIXED: \
   case CONST_VECTOR

and remove the mark_jump_label_1 cases.

The name "CASE_CONST_SCALAR_INT" matches "SCALAR_INT_MODE_P",
CASE_CONST_UNIQUE is solely for the equality functions above.

2) As for (1), but keep the mark_jump_label_1 cases and
rename CASE_CONST_UNIQUE to CASE_CONST_LEAF so that mark_jump_label_1
can use it.  This implies that all "leaf" CONST_*s (i.e. those without
rtx operands) will always be hashed to give pointer equality.

I don't like that assumption, and CASE_CONST_UNIQUE feels like a better
abstraction, so I prefer (1) over (2).

For avoidance of doubt, this isn't an approval or anything.
It definitely needs to be submitted upstream.  (Yeah, yeah,
sorry for nagging. :-))

Richard
==================