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
is why I was skimming further over existing code where I have found this
in combine.cc:

      /* (unsigned) < 0x80000000 is equivalent to >= 0.  */
      else if (is_a <scalar_int_mode> (mode, &int_mode)
               && GET_MODE_PRECISION (int_mode) - 1 < HOST_BITS_PER_WIDE_INT
               && ((unsigned HOST_WIDE_INT) const_op
                   == HOST_WIDE_INT_1U << (GET_MODE_PRECISION (int_mode) - 1)))
        {

The expression of the if-statement is a contradiction rendering the then-part
unreachable unless you mask out the upper bits of the sign-extended
unsigned integer const_op (as proposed in the inlined patch):

      ((unsigned HOST_WIDE_INT) const_op & GET_MODE_MASK (int_mode))

This is why I got a bit uncertain and hoped to get some feedback whether
my intuition is correct or not.  Meanwhile I also found a comment in
the internals book at "14.7 Constant Expression Types" where we have:

   "Constants generated for modes with fewer bits than in HOST_WIDE_INT
    must be sign extended to full width (e.g., with gen_int_mode).
    [...]
    Note however that values are neither inherently signed nor
    inherently unsigned; where necessary, signedness is determined by
    the rtl operation instead."

At least this and the assert statement document that the normal form of
a CONST_INT is kind of special w.r.t. unsigned integers.  Is there
anyone who can shed some light on _why_ such a normal form was chosen?

Independent of why such a normal form was chosen, this patch restores
the normal form and solves the bootstrap problem for Loongarch.

Cheers,
Stefan

Reply via email to