Comment 10 for bug 803232

I'm testing the attached patch overnight and tomorrow. If all goes well,
I'll submit it on Thursday.

There seems to be a bit of history here. The current choose_reload_regs
code reads:

    if (byte == 0)
      need_mode = mode;
    else
      need_mode
        = smallest_mode_for_size
          (GET_MODE_BITSIZE (mode) + byte * BITS_PER_UNIT,
    GET_MODE_CLASS (mode) == MODE_PARTIAL_INT
    ? MODE_INT : GET_MODE_CLASS (mode));

    if ((GET_MODE_SIZE (GET_MODE (last_reg))
         >= GET_MODE_SIZE (need_mode))

Note that this is the only use of need_mode. I don't believe the mode
that is being calculated here is fundamental in any way, or that it's
used later in the reload process. We have already checked that the mode
change is allowed:

#ifdef CANNOT_CHANGE_MODE_CLASS
    /* Verify that the register it's in can be used in
       mode MODE. */
    && !REG_CANNOT_CHANGE_MODE_P (REGNO (reg_last_reload_reg[regno]),
      GET_MODE (reg_last_reload_reg[regno]),
      mode)
#endif

and have already calculated which hard register we would need to
use after the mode change:

    i = REGNO (last_reg);
    i += subreg_regno_offset (i, GET_MODE (last_reg), byte, mode);

So once we have verified that the register is suitable, we can (and do)
simply use register I in mode MODE.

I think the current mode is a historical left-over. Back in 2000 this code
was a simple check that the old register entirely encompassed the new one:

    i = REGNO (last_reg) + word;
    last_class = REGNO_REG_CLASS (i);
    if ((GET_MODE_SIZE (GET_MODE (last_reg))
         >= GET_MODE_SIZE (mode) + word * UNITS_PER_WORD)

The register we were interested in was (reg:MODE I), and this check made
sure that the old reload register defined every bit of (reg:MODE I).
When CLASS_CANNOT_CHANGE_SIZE was introduced, the code became:

    i = REGNO (last_reg) + word;
    last_class = REGNO_REG_CLASS (i);
    if (
#ifdef CLASS_CANNOT_CHANGE_SIZE
        (TEST_HARD_REG_BIT
         (reg_class_contents[CLASS_CANNOT_CHANGE_SIZE], i)
         ? (GET_MODE_SIZE (GET_MODE (last_reg))
     == GET_MODE_SIZE (mode) + word * UNITS_PER_WORD)
         : (GET_MODE_SIZE (GET_MODE (last_reg))
     >= GET_MODE_SIZE (mode) + word * UNITS_PER_WORD))
#else
        (GET_MODE_SIZE (GET_MODE (last_reg))
         >= GET_MODE_SIZE (mode) + word * UNITS_PER_WORD)
#endif

But I think this was bogus. The new size of the register was:

   GET_MODE_SIZE (mode)

rather than:

   GET_MODE_SIZE (mode) + word * UNITS_PER_WORD

Maybe something like:

   word == 0 && GET_MODE_SIZE (mode) == GET_MODE_SIZE (GET_MODE (last_reg))

would have been more accurate. Anyway, CLASS_CANNOT_CHANGE_SIZE proved
to be too limited, so it was replaced with CLASS_CANNOT_CHANGE_MODE.
The code above then became:

    need_mode = smallest_mode_for_size ((word+1) * UNITS_PER_WORD,
            GET_MODE_CLASS (mode));

    if (
#ifdef CLASS_CANNOT_CHANGE_MODE
        (TEST_HARD_REG_BIT
         (reg_class_contents[(int) CLASS_CANNOT_CHANGE_MODE], i)
         ? ! CLASS_CANNOT_CHANGE_MODE_P (GET_MODE (last_reg),
             need_mode)
         : (GET_MODE_SIZE (GET_MODE (last_reg))
     >= GET_MODE_SIZE (need_mode)))
#else
        (GET_MODE_SIZE (GET_MODE (last_reg))
         >= GET_MODE_SIZE (need_mode))
#endif

with need_mode providing a mode of the same size as the then-preexisting
size check. I think this mode is bogus for the same reason, and in 2005
I changed the third mode argument from "need_mode" to "mode":

    http://gcc.gnu.org/ml/gcc-patches/2005-02/msg01665.html

That patch also fixed the smallest_mode_for_size argument so that it
was a bit count rather than a byte count. Unfortunately, it seems
I failed to realise that need_mode was in fact completely meaningless,
and should have just been removed instead. Indeed, the bit->byte fix
exposed a bug very like this one on s390:

    http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01226.html

Ulrich wisely said:

    As real fix, I think the computation of a "needed mode" may be
    completely superfluous in the first place; instead, the first
    clause of the 'if' could just be replaced by

      if ((GET_MODE_SIZE (GET_MODE (last_reg))
    >= GET_MODE_SIZE (mode) + byte)

to which I whined:

    I remember wondering about this too. Unfortunately, there are no
    comments at all explaining what the check is actually supposed to
    do, or what "need_mode" is suppsoed to be, so I thought at the time
    it was best to leave things be.

But we were in release-paranoia mode, and that real fix never happened.
I agree that removing need_mode is the right fix for that s390 PR,
and for this NEON one.

Richard