Kenneth Zadeck <zad...@naturalbridge.com> writes:
>> Or do you want to optimize encoding like for CONST_INT (and unlike
>> CONST_DOUBLE)?  I doubt the above packs nicely into rtx_def?
>> A general issue of it though - we waste 32bits on 64bit hosts in
>> rtx_def between the bits and the union.  Perfect place for num_elem
>> (if you really need it) and avoids another 4 byte hole.  Similar issue
>> exists in rtvec_def.
> I am going to let richard answer this.

I agree this would be nice, but (stating the obvious) only on targets
where there is a hole.  I suppose that translates to something like:

#if (HOST_BITS_PER_PTR > HOST_BITS_PER_INT \
     || HOST_BITS_PER_WIDE_INT > HOST_BITS_PER_INT)
#define RTL_LAYOUT 1
#else
#define RTL_LAYOUT 2
#endif

where HOST_BITS_PER_PTR would have to be set by a new configure-time test.
RTL_LAYOUT == 1 would put the CONST_WIDE_INT vector length in rtx_def itself,
probably as part of a new union, while RTL_LAYOUT == 2 would keep
it in its current place.

E.g. between:

  unsigned return_val : 1;

and:

  /* The first element of the operands of this rtx.
     The number of operands and their types are controlled
     by the `code' field, according to rtl.def.  */
  union u {

have:

#if RTL_LAYOUT == 1
  union {
    int num_elem;
  } u2;
#endif

And:

struct GTY((variable_size)) hwivec_def {
#if RTL_LAYOUT == 2
  int num_elem;
#endif
  HOST_WIDE_INT elem[1];
};

>> +#if TARGET_SUPPORTS_WIDE_INT
>> +
>> +/* Match CONST_*s that can represent compile-time constant integers.  */
>> +#define CASE_CONST_SCALAR_INT \
>> +   case CONST_INT: \
>> +   case CONST_WIDE_INT
>> +
>>
>> I regard this abstraction is mostly because the transition is not finished.
>> Not sure if seeing CASE_CONST_SCALAR_INT, CASE_CONST_UNIQUE (?),
>> CASE_CONST_ANY adds more to the confusion than spelling out all of them.
>> Isn't there sth like a tree-code-class for RTXen?  That would catch
>> CASE_CONST_ANY, no?

Not as things stand.  All constants are classified as RTX_CONST_OBJ,
which includes run-time constants like SYMBOL_REF as well.  Whether it
makes sense to change the classification is really a separate question
from this patch.  Besides, the other cases in the affected switch
statements aren't necessarily going to divide into classes either,
so we might still need CASE_CONST_ANY.

I'd like to keep these macros even after the conversion.  They make
it more obvious what property is being tested, instead of having various
subsets of the CONST_* codes included by name.  Reviewing the patch that
introduced the macros showed up instances where the existing switch
statements were wrong.

>> @@ -3081,6 +3081,8 @@ commutative_operand_precedence (rtx op)
>>     /* Constants always come the second operand.  Prefer "nice" constants.  
>> */
>>     if (code == CONST_INT)
>>       return -8;
>> +  if (code == CONST_WIDE_INT)
>> +    return -8;
>>     if (code == CONST_DOUBLE)
>>       return -7;
>>     if (code == CONST_FIXED)
>>
>> I think it should be the same as CONST_DOUBLE which it "replaces".
>> Likewise below, that avoids code generation changes (which there should
>> be none for a target that is converted, right?).
> not clear because it does not really replace const double - remember 
> that const double still holds fp stuff.    another one for richard to 
> comment on.

I think it should be the same as CONST_INT.  Whether an integer
fits into a HWI or not shouldn't affect its precedence.

>> +/* This is the maximal size of the buffer needed for dump.  */
>> +const int MAX = (MAX_BITSIZE_MODE_ANY_INT / 4
>> +                + MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 32);
>>
>> I'd prefer a target macro for this, not anything based off
>> MAX_BITSIZE_MODE_ANY_INT just to allow no-op transition of a
>> target to wide-ints (which IMHO should be the default for all targets,
>> simplifying the patch).

Kenny didn't comment on this, but I disagree.  Part of the process of
"converting" a port is to ween it off any assumptions about the number
of HWIs.

Richard

Reply via email to