[PATCH, ARM] Further improve stack usage on sha512 (PR 77308)

[Bernd Edlinger]

Hi Wilco,

On 11/30/16 18:01, Bernd Edlinger wrote:
> I attached the completely untested follow-up patch now, but I would
> like to post that one again for review, after I applied my current
> patch, which is still waiting for final review (please feel pinged!).
>
>
> This is really exciting...
>
>


when testing the follow-up patch I discovered a single regression
in gcc.dg/fixed-point/convert-sat.c that was caused by a mis-compilation
of the libgcc function __gnu_satfractdasq.

I think it triggerd a latent bug in the carryin_compare patterns.

everything is as expected until reload.  First what is left over
of a split cmpdi_insn followed by a former cmpdi_unsigned, if the
branch is not taken.

(insn 109 10 110 2 (set (reg:CC 100 cc)
         (compare:CC (reg:SI 0 r0 [orig:124 _10 ] [124])
             (const_int 0 [0]))) 
"../../../gcc-trunk/libgcc/fixed-bit.c":785 196 {*arm_cmpsi_insn}
      (nil))
(insn 110 109 13 2 (parallel [
             (set (reg:CC 100 cc)
                 (compare:CC (reg:SI 1 r1 [orig:125 _10+4 ] [125])
                     (const_int -1 [0xffffffffffffffff])))
             (set (reg:SI 3 r3 [123])
                 (minus:SI (plus:SI (reg:SI 1 r1 [orig:125 _10+4 ] [125])
                         (const_int -1 [0xffffffffffffffff]))
                     (ltu:SI (reg:CC_C 100 cc)
                         (const_int 0 [0]))))
         ]) "../../../gcc-trunk/libgcc/fixed-bit.c":785 32 
{*subsi3_carryin_compare_const}
      (nil))
(jump_insn 13 110 31 2 (set (pc)
         (if_then_else (ge (reg:CC_NCV 100 cc)
                 (const_int 0 [0]))
             (label_ref:SI 102)
             (pc))) "../../../gcc-trunk/libgcc/fixed-bit.c":785 204 
{arm_cond_branch}
      (int_list:REG_BR_PROB 6400 (nil))

(note 31 13 97 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(note 97 31 114 3 NOTE_INSN_DELETED)
(insn 114 97 113 3 (set (reg:SI 2 r2 [orig:127+4 ] [127])
         (const_int -1 [0xffffffffffffffff])) 
"../../../gcc-trunk/libgcc/fixed-bit.c":831 630 {*arm_movsi_vfp}
      (expr_list:REG_EQUIV (const_int -1 [0xffffffffffffffff])
         (nil)))
(insn 113 114 107 3 (set (reg:SI 3 r3 [126])
         (const_int 2147483647 [0x7fffffff])) 
"../../../gcc-trunk/libgcc/fixed-bit.c":831 630 {*arm_movsi_vfp}
      (expr_list:REG_EQUIV (const_int 2147483647 [0x7fffffff])
         (nil)))
(insn 107 113 108 3 (set (reg:CC 100 cc)
         (compare:CC (reg:SI 1 r1 [orig:125 _10+4 ] [125])
             (reg:SI 2 r2 [orig:127+4 ] [127]))) 
"../../../gcc-trunk/libgcc/fixed-bit.c":831 196 {*arm_cmpsi_insn}
      (nil))


Note that the CC register is not really set as implied by insn 110,
because the C flag depends on the comparison of r1, 0xFFFF and the
carry flag from insn 109.  Therefore in the postreload pass the
insn 107 appears to be unnecessary, as if should compute
exactly the same CC flag, as insn 110, i.e. not dependent on
previous CC flag.  I think all carryin_compare patterns are
wrong because they do not describe the true value of the CC reg.

I think the CC reg is actually dependent on left, right and CC-in
value, as in the new version of the patch it must be a computation
in DI mode without overflow, as in my new version of the patch.

I attached an update of the followup patch which is not yet adjusted
on your pending negdi patch.  Reg-testing is no yet done, but the
mis-compilation on libgcc is fixed at least.

What do you think?


Thanks
Bernd.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-pr77308-5.diff
Type: text/x-patch
Size: 15481 bytes
Desc: patch-pr77308-5.diff
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20161208/de6d5058/attachment.bin>