On Wed, Jun 25, 2014 at 05:21:08PM +1000, Kugan wrote:
> The problem with SRP_POINTER 0, SRP_SIGNED 1, SRP_UNSIGNED 2,
> SRP_SIGNED_AND_UNSIGNED 3 (as I understand) is that, it will be
> incompatible with TYPE_UNSIGNED (tree) and defines of
> POINTER_EXTEND_UNSIGNED values. We will have to then translate while
> setting to SRP_* values . Also SUBREG_PROMOTED_SIGNED_P is now checked
> in some cases for != 0 (meaning SRP_POINTER or SRP_UNSIGNED) and in some
> cases > 0 (meaning SRP_UNSIGNED).
>
> Since our aim is to perform single bit checks, why don’t we just use
> this representation internally (i.e. _rtx->unchanging = 1 if SRP_SIGNED
> and _rtx->volatil = 1 if SRP_UNSIGNED). As for SUBREG_PROMOTED_SIGNED_P,
> we still have to return -1 or 1 depending on SRP_POINTER or SRP_UNSIGNED.
Why don't you make SUBREG_PROMOTED_UNSIGNED_P just return 0/1 (i.e. the
single bit), and for places where it would like to match both
SRP_UNSIGNED and SRP_POINTER use SUBREG_PROMOTED_GET () & SRP_UNSIGNED
or so?
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -1448,8 +1448,11 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x,
> enum rtx_code code,
> || byte_vtrue != byte_vfalse
> || (SUBREG_PROMOTED_VAR_P (vtrue)
> != SUBREG_PROMOTED_VAR_P (vfalse))
> - || (SUBREG_PROMOTED_UNSIGNED_P (vtrue)
> - != SUBREG_PROMOTED_UNSIGNED_P (vfalse)))
> + || ((SUBREG_PROMOTED_UNSIGNED_P (vtrue)
> + != SUBREG_PROMOTED_UNSIGNED_P (vfalse))
> + && (SUBREG_PROMOTED_SIGNED_P (vtrue)
> + != SUBREG_PROMOTED_SIGNED_P (vfalse))))
Shouldn't this be SUBREG_PROMOTED_GET (vtrue) != SUBREG_PROMOTED_GET (vfalse) ?
> +const unsigned int SRP_POINTER = -1;
> +const unsigned int SRP_SIGNED = 0;
Inconsistent whitespace, just use space instead of multiple spaces and/or
tabs.
> +const unsigned int SRP_UNSIGNED = 1;
> +const unsigned int SRP_SIGNED_AND_UNSIGNED = 2;
> +/* Predicate to check if RTX of SUBREG_PROMOTED_VAR_P() is promoted
> + for SIGNED type. */
> +#define SUBREG_PROMOTED_SIGNED_P(RTX) \
> + (RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_SIGNED_P", (RTX), SUBREG)->unchanging
> == 1)
Why the " == 1" ?
> +
> +/* Predicate to check if RTX of SUBREG_PROMOTED_VAR_P() is promoted
> + for UNSIGNED type. In case of SRP_POINTER, SUBREG_PROMOTED_UNSIGNED_P
> + returns -1 as this is in most cases handled like unsigned extension,
> + except for generating instructions where special code is emitted for
> + (ptr_extend insns) on some architectures. */
> #define SUBREG_PROMOTED_UNSIGNED_P(RTX) \
> - ((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_UNSIGNED_P", (RTX), SUBREG)->volatil) \
> - ? -1 : (int) (RTX)->unchanging)
> + ((((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_UNSIGNED_P", (RTX),
> SUBREG)->volatil)\
> + + (RTX)->unchanging) == 0) ? -1 : ((RTX)->volatil == 1))
> +
> +/* Checks if RTX of SUBREG_PROMOTED_VAR_P() is promotd for given SIGN. */
> +#define SUBREG_CHECK_PROMOTED_SIGN(RTX, SIGN) \
Use space rather than tab. Also, why do we need this macro?
Can't you just use SUBREG_PROMOTED_GET () == sign ? I mean, sign in that
case is typically just 0 or 1.
Jakub
