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

Reply via email to