This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Analysis of high priority PR c/2454


Following Mark Mitchell's request to investigate the outstanding
high priority PRs, I've spent a little time looking into the
cause of PR c/2454.  As described in GNATS, the testcase 20011223-1.c
is now fixed on most platforms thanks to Joseph Myers' patch, but
it still fails on a few targets including v850, sh and mn10300.

Using v850 as a test system, I've tracked down the backend issue
to combine.c.  The v850 synthesizes signed extensions using shifts
and so generates RTL of the following form:

(insn 31 30 32 (set (reg:SI 47)
        (ashift:SI (subreg:SI (reg:QI 46) 0)
            (const_int 24 [0x18]))) 51 {ashlsi3} (insn_list 30 (nil))
    (expr_list:REG_DEAD (reg:QI 46)
        (nil)))

(insn 32 31 34 (set (reg:SI 45)
        (ashiftrt:SI (reg:SI 47)
            (const_int 24 [0x18]))) 53 {ashrsi3} (insn_list 31 (nil))
    (expr_list:REG_DEAD (reg:SI 47)
        (expr_list:REG_EQUAL (sign_extend:SI (reg:QI 46))
            (nil))))


The issue is that combine merges the two instructions together and
calls simplify_shift_const on the following:

    (ashiftrt:SI (ashift:SI (subreg:SI (reg:QI 46) 0)
                            (const_int 24 [0x18]))
                 (const_int 24 [0x18]))

which it manages to reduce to just the paradoxical subreg:

    (subreg:SI (reg:QI 46))

Which seems very broken to me.  Tracing the problem further
it's because num_sign_bit_copies believes that the top 25 bits
of "(subreg:SI (reg:QI 46))" are always the same, because the
v850 backend defines WORD_REGISTER_OPERATIONS and LOAD_EXTEND_OP
as always SIGN_EXTEND.

The relevant code is at line 8560 of combine.c:

#ifdef WORD_REGISTER_OPERATIONS
#ifdef LOAD_EXTEND_OP
      /* For paradoxical SUBREGs on machines where all register operations
         affect the entire register, just look inside.  Note that we are
         passing MODE to the recursive call, so the number of sign bit copies
         will remain relative to that mode, not the inner mode.  */

      /* This works only if loads sign extend.  Otherwise, if we get a
         reload for the inner part, it may be loaded from the stack, and
         then we lose all sign bit copies that existed before the store
         to the stack.  */

      if ((GET_MODE_SIZE (GET_MODE (x))
           > GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))))
          && LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) == SIGN_EXTEND)
        return num_sign_bit_copies (SUBREG_REG (x), mode);
#endif
#endif

If this code isn't executed the result of num_sign_bit_copies is 1,
and the test case should be fixed.  However, I can't work out if its
the code above that's broken, or the v850 backend for incorrectly
defining LOAD_EXTEND_OP.

I hope this helps.  I'll shortly be submitting a patch to combine.c
to clean up all of the compilation warnings, as its our friend
num_sign_bit_copies above that appears to be the cause of most of
them!

Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]