I am certainly not going to check it in if there are any issues with the patch. However, this was basically a trivial lexicographical cleanup, and if no one has any comments on it after a reasonable amount of time, then i do feel this is ok. Obviously if anyone has any comments. that is completely a different issue.

I named it this way CASE_CONST_SCALAR_INTEGER because i need to introduce in the next patch a predicate that looks like

/* Predicate yielding true iff X is an rtx for a integer const.  */
#if TARGET_SUPPORTS_WIDE_INT == 1
#define CONST_INTEGER_P(X) \
  (CONST_INT_P (X) || CONST_WIDE_INT_P (X))
#else
#define CONST_INTEGER_P(X) \
  (CONST_INT_P (X) || CONST_DOUBLE_AS_INT_P (X))
#endif

for all of the rtxs that represent an integer. And this name was consistent with that. It may be that you have a suggestion for the name of predicate as well but it seemed to make more sense to have the rtxs be consistent rather than rtx/mode consistent.

kenny

On 08/21/2012 12:56 PM, Richard Sandiford wrote:
Kenneth Zadeck <zad...@naturalbridge.com> writes:
I plan to commit this in a few days unless someone has some comments.
This is a mostly trivial patch and the changes from that are Richard
Sandiford's and he is an rtl maintainer.
Please don't do this.  Patches need to be sent for review in their
final form.  Obviously, having got this far with the patch, you're free
to beat me up if I don't review it. :-)

Anyway, please do call it CASE_CONST_SCALAR_INT rather than
CASE_CONST_SCALAR_INTEGER.  Like I said in my original mail,
CASE_CONST_SCALAR_INT chimes nicely with SCALAR_INT_MODE_P, etc.,
and (as I didn't say) it'd be better not to have two spellings
of the same thing.

diff -upNr '--exclude=.svn' gccBaseline/gcc/combine.c gccWCase/gcc/combine.c
--- gccBaseline/gcc/combine.c   2012-08-17 09:35:24.802195795 -0400
+++ gccWCase/gcc/combine.c      2012-08-20 15:43:34.659362244 -0400
@@ -531,12 +531,10 @@ find_single_use_1 (rtx dest, rtx *loc)

    switch (code)
      {
-    case CONST_INT:
      case CONST:
      case LABEL_REF:
      case SYMBOL_REF:
-    case CONST_DOUBLE:
-    case CONST_VECTOR:
+    CASE_CONST_UNIQUE:
      case CLOBBER:
        return 0;

@@ -12788,10 +12786,8 @@ mark_used_regs_combine (rtx x)
      {
      case LABEL_REF:
      case SYMBOL_REF:
-    case CONST_INT:
      case CONST:
-    case CONST_DOUBLE:
-    case CONST_VECTOR:
+    CASE_CONST_UNIQUE:
      case PC:
      case ADDR_VEC:
      case ADDR_DIFF_VEC:
These were supposed to be CASE_CONST_ANY.  The omission of CONST_FIXED
looks like an oversight.

    switch (code)
      {
-    case CONST_INT:
-    case CONST_DOUBLE:
-    case CONST_FIXED:
+    CASE_CONST_UNIQUE:
      case SYMBOL_REF:
      case CONST:
      case LABEL_REF:
This was suppsoed to be CASE_CONST_ANY too.  The omission of CONST_VECTOR
looks like an oversight.

+/* Match CONST_*s for which pointer equality corresponds to value
+equality.  */
Should be:

/* Match CONST_*s for which pointer equality corresponds to value equality.  */

(probably an artefact of my work mailer, sorry)

+
+
+
Rather a lot of whitespace there.  One line seems enough, since we're
just before the definition of CONST_INT_P.

OK with those changes, thanks.

Richard

Reply via email to