Re: PING^5: [PATCH] rtl-optimization/110939 Really fix narrow comparison of memory and constant

[Jeff Law <jeffreyalaw@gmail.com>]

On 9/19/23 10:06, Stefan Schulze Frielinghaus wrote:
> Since this patch is sitting in the queue for quite some time and (more
> importantly?) solves a bootstrap problem let me reiterate:
> 
> While writing the initial commit 7cdd0860949c6c3232e6cff1d7ca37bb5234074c
> and the subsequent (potential) fix 41ef5a34161356817807be3a2e51fbdbe575ae85
> I was not aware of the fact that the normal form of a CONST_INT,
> representing an unsigned integer with fewer bits than in HOST_WIDE_INT,
> is a sign-extended version of the unsigned integer.  This invariant is
> checked in rtl.h where we have at line 2297:
> 
>      case CONST_INT:
>        if (precision < HOST_BITS_PER_WIDE_INT)
>          /* Nonzero BImodes are stored as STORE_FLAG_VALUE, which on many
>             targets is 1 rather than -1.  */
>          gcc_checking_assert (INTVAL (x.first)
>                               == sext_hwi (INTVAL (x.first), precision)
>                               || (x.second == BImode && INTVAL (x.first) == 1));
> 
> This was pretty surprising and frankly speaking unintuitive to me which

At the heart of the matter (as I think Eric recently mentioned) is that 
CONST_INT objects do not have a mode.  So consider something like 
(const_int 32768) on a 32bit host.

In modes wider than HImode is does exactly what you'd expect.  But if 
you tried to use it in a HImode context, then it's actually invalid as 
it needs to be sign extended resulting in (const_int -32768).  That's 
what gen_int_mode handles for you -- you provide a mode for the context 
in which the constant will be used and you get back suitable RTL.

This actually mimicks what some targets do in hardware for 32 bit 
objects being held in 64bit registers.

Anyway, that's the background.  It is highly likely there are bugs where 
we fail to use gen_int_mode when we should or in some tests for when an 
optimization may or may not be applicable.  So I wouldn't be surprised 
at all if you were to grub around and find cases that aren't properly 
handled -- particularly of the missed-optimization variety.


>
> 
> Independent of why such a normal form was chosen, this patch restores
> the normal form and solves the bootstrap problem for Loongarch.
Digging it out and looking at it momentarily.  It just keep falling off 
the end of the TODO list day-to-day.  Sorry for that, particularly since 
you're probably getting bugged by folks looking to restore builds/test 
results.

jeff