Re: Zero/Sign extension elimination using value ranges

[Kugan <kugan dot vivekanandarajah at linaro dot org>]

On 23/05/14 07:23, Richard Henderson wrote:
> On 05/22/2014 03:12 AM, Jakub Jelinek wrote:
>> No way.  SUBREG_PROMOTED_UNSIGNED_P right now resides in two separate bits,
>> volatil and unchanging.  Right now volatile != 0, unchanging ignored
>> is -1, volatile == 0, then the value is unchanging.
>> What I meant is change this representation, e.g. to
>> x->volatil * 2 + x->unchanging - 1
>> so you can represent the values -1, 0, 1, 2 in there.
>> Of course, adjust SUBREG_PROMOTED_UNSIGNED_SET correspondingly too.
>> As SUBREG_PROMOTED_UNSIGNED_P is only valid if SUBREG_PROMOTED_VAR_P,
>> I'd hope that you don't need to care about what 0, 0 in those bits
>> means, because everything should actually SUBREG_PROMOTED_UNSIGNED_SET
>> around setting SUBREG_PROMOTED_VAR_P to non-zero.
> 
> It would be helpful to redo these, now that we don't simply have a tri-state value.
> 
> const unsigned int SRP_POINTER  = 0;
> const unsigned int SRP_SIGNED   = 1;
> const unsigned int SRP_UNSIGNED = 2;
> 
> #define SUBREG_PROMOTED_SET(RTX, VAL)                          \
> do {                                                           \
>   rtx const _rtx = RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_SET",     \
>                                     (RTX), SUBREG);            \
>   unsigned int _val = (VAL);                                   \
>   _rtx->volatil = _val;                                        \
>   _rtx->unchanging = _val >> 1;                                \
> } while (0)
> 
> #define SUBREG_PROMOTED_GET(RTX) \
>   ({ const rtx _rtx = RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_GET",  \
>                                        (RTX), SUBREG);         \
>      _rtx->volail + _rtx->unchanging * 2;                      \
>   })
> 
> The bits are arranged such that e.g.
> 
>   SUBREG_PROMOTED_GET (x) & SRP_UNSIGNED
> 
> is meaningful.  For conciseness, we'd probably want
> 
> SUBREG_PROMOTED_POINTER_P
> SUBREG_PROMOTED_UNSIGNED_P
> SUBREG_PROMOTED_SIGNED_P
> 
> as boolean macros.  I dunno if "both" (whatever you want to call that) is used
> enough to warrant its own macro.  I can more often see this being used when
> examining a given ZERO_/SIGN_EXTEND rtx, so "both" probably won't come up.


Thanks for the information.

#define SUBREG_PROMOTED_UNSIGNED_P(RTX)        \
  ({ const rtx _rtx = RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_UNSIGNED_P",
(RTX), SUBREG);   \
  (((_rtx->volatil + _rtx->unchanging) == 0) ? -1 : (_rtx->volatil == 1));})
when I tried this macros, I started getting "warning: ISO C++ forbids
braced-groups within expressions [-Wpedantic]", Therefore I changed it to

#define SUBREG_PROMOTED_UNSIGNED_P(RTX)	\
  ((((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_UNSIGNED_P", (RTX),
SUBREG)->volatil)\
     + (RTX)->unchanging) == 0) ? -1 : ((RTX)->volatil == 1))

I also kept the SRP_POINTER, SRP_SIGNED etc. as below. This is similar
to the sign values we get from tree and same as what is used currently.
We therefore don’t have to translate between them.

I am however, changing the internal values (of volatil and unchanging)
similar to what we will get with SRP_SIGNED=1 and SRP_UNSIGNED = 2.
Attached patch has the rest of the modifications. Regression tested on
arm and x86_64. In arm there is still one failure
(gcc.dg/fixed-point/convert-sat.c).

const unsigned int SRP_POINTER  = -1;
const unsigned int SRP_SIGNED   = 0;
const unsigned int SRP_UNSIGNED = 1;
const unsigned int SRP_SIGNED_AND_UNSIGNED = 2;


#define SUBREG_PROMOTED_SET(RTX, VAL)		                \
do {							        \
  rtx const _rtx = RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_SET",	\
                                    (RTX), SUBREG);		\
  switch ((VAL))						\
  {								\
    case SRP_POINTER:						\
      _rtx->volatil = 0;					\
      _rtx->unchanging = 0;					\
      break;							\
    case SRP_SIGNED:						\
      _rtx->volatil = 0;					\
      _rtx->unchanging = 1;					\
      break;							\
    case SRP_UNSIGNED:						\
      _rtx->volatil = 1;					\
      _rtx->unchanging = 0;					\
      break;							\
    case SRP_SIGNED_AND_UNSIGNED:				\
      _rtx->volatil = 1;					\
      _rtx->unchanging = 1;					\
      break;							\
   }								\
 } while (0)


#define SUBREG_PROMOTED_GET(RTX)	\
  (2 * ((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_GET", (RTX), SUBREG))->volatil)\
   + (RTX)->unchanging - 1)

#define SUBREG_PROMOTED_SIGNED_P(RTX)	\
  ((((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_SIGNED_P", (RTX), SUBREG)->volatil)\
     + (RTX)->unchanging) == 0) ? 0 : ((RTX)->unchanging == 1))

 #define SUBREG_PROMOTED_UNSIGNED_P(RTX)	\
  ((((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_UNSIGNED_P", (RTX),
SUBREG)->volatil)\
     + (RTX)->unchanging) == 0) ? -1 : ((RTX)->volatil == 1))

#define	SUBREG_CHECK_PROMOTED_SIGN(RTX, SIGN) \
     ((SIGN) ? SUBREG_PROMOTED_UNSIGNED_P((RTX))\
	     : SUBREG_PROMOTED_SIGNED_P((RTX)))	\

Does this look reasonable?

Thanks,
Kugan

Attachment: p.txt
Description: Text document