> if plus_constant _knows_ that something can be wrapped in a CONST,
> simplify_binary_operation should have given us the CONST to begin with.
> Also, the only cases that plus_constant can handle are CONST,
> SYMBOL_REF and LABEL_REF, all of which satisfy CONSTANT_P.
> So the new form ought to be dead on two counts.

Yes, and in the other case too:

      ops[i - 1].op = gen_rtx_MINUS (mode, ops[i - 1].op, ops[i].op);
      ops[i - 1].op = plus_constant (ops[i - 1].op, 0);

plus_constant won't understand the MINUS, and won't generate a CONST.

Still, having a new target hook for this seems overkill.  For example,
since ports do have to deal with complicated constants when they expand
moves, and since some of them already look inside CONSTs in their
LEGITIMATE_CONSTANT_P, another possibility to throw in the air is
something like (better names welcome...)

rtx
avoid_terrible_constants (rtx x)
{
  if (!CONSTANT_P (x))
    x = gen_rtx_CONST (x);

  /* If the target's move expanders will take care of it,
     it must not be that bad.  */
  icode = optab_handler (mov_optab, GET_MODE (x))->insn_code;
  if (*insn_data[icode].operand[1].predicate (x, GET_MODE (x)))
    return x;

  return NULL;
}

In case of cris, the predicate goes into general_operand, which does

  if (CONSTANT_P (op))
    return ((GET_MODE (op) == VOIDmode || GET_MODE (op) == mode
             || mode == VOIDmode)
            && (! flag_pic || LEGITIMATE_PIC_OPERAND_P (op))
            && LEGITIMATE_CONSTANT_P (op));

H-P can check for the problematic case inside his LEGITIMATE_CONSTANT_P
(*), or add a move expander for it.

  (*) but then does this mean the documentation for L_C_P is obsolete,
  and returning 1 is not necessarily a good thing to do for targets with
  sections?  Maybe there is a better definition that can be the default?

Anyway, at least how to use this function is pretty obvious:

                    tem_rhs = GET_CODE (rhs) == CONST ? XEXP (rhs, 0) : rhs;
                    tem = simplify_binary_operation (ncode, mode,
tem_lhs, tem_rhs);

-                   if (tem && !CONSTANT_P (tem))
-                     tem = gen_rtx_CONST (GET_MODE (tem), tem);
+                   if (tem)
+                     tem = avoid_terrible_constants (tem);
                  }
                else
                  tem = simplify_binary_operation (ncode, mode, lhs, rhs);


...


       && CONSTANT_P (ops[i].op)
       && GET_CODE (ops[i].op) == GET_CODE (ops[i - 1].op))
     {
-      ops[i - 1].op = gen_rtx_MINUS (mode, ops[i - 1].op, ops[i].op);
-      ops[i - 1].op = gen_rtx_CONST (mode, ops[i - 1].op);
-      if (i < n_ops - 1)
-       ops[i] = ops[i + 1];
-      n_ops--;
+      rtx x;
+      x = gen_rtx_MINUS (mode, ops[i - 1].op, ops[i].op);
+      x = avoid_terrible_constants (x);
+      if (x)
+       {
+          ops[i - 1].op = x;
+          if (i < n_ops - 1)
+           ops[i] = ops[i + 1];
+          n_ops--;
+        }
     }

   if (n_ops > 1


I'm absolutely unsure that this is the way to go; but it has two
advantages: 1) not leaking really bad constants outside simplify-rtx.c;
2) it makes clear how to fix bugs -- you restrict
LEGITIMATE_CONSTANT_P/LEGITIMATE_PIC_OPERAND_P or add a move expander.

Paolo

Reply via email to