search

jkmcnk / sx-gcc

The GNU Compiler Collection port to NEC SX CPU architecture.
GNU General Public License v2.0
0 stars 2 forks source link

unsigned modulo calculation fails #89

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
testcase gcc.c-torture/execute/990106-2.c

((unsigned)-1)%1234 == 4294967295%1234 == 0 according to our gcc port.
actually, it should be 679.

Original issue reported on code.google.com by jmoc...@gmail.com on 12 Jan 2009 at 1:01

GoogleCodeExporter commented 8 years ago 
the problem occurs with -O>0.

it is caused by the compiler (which uses float division to calculate USI modulo 
due
to lack of int division on SX) putting ((unsigned)-1) in the register via fltx 
with
an immediate operand, which actually stores ((float)-1) in the register, 
instead of
(float)((unsigned)-1). thus, we actually calculate -1 mod 1234 instead of what 
we
should calculate.

with -O0, compiler uses libgcc2 __floatunsidf for converting -1 to float, which 
is
the Right Way To Go(tm).

__floatunsidf is used for 1234 in both cases, as 1234 is not suitable for an 
immediate.

Original comment by jmoc...@gmail.com on 12 Jan 2009 at 1:52

GoogleCodeExporter commented 8 years ago 
this testcase suffers from other problems, not just the faulty unsigned->float
conversion: with -O1, lots of initial insns at the beginning of calc_mp() code 
are
optimized away, but in a completely wrong way. in the final assembly, the 
following 2
insns are left out of initialy 18 insns at the start of calc_mp():

#(insn 6 8 7 (set (reg/v:SI 9 s9 [orig:135 mod ] [135])
#        (mem/c/i:SI (plus:DI (reg/f:DI 3 s3)
#                (const_int 12 [0xc])) [0 mod+0 S4 A32])) 58 {movsi_general} 
(nil)
#    (expr_list:REG_DEAD (reg/f:DI 3 s3)
#        (expr_list:REG_EQUIV (mem/c/i:SI (plus:DI (reg/f:DI 3 s3)
#                    (const_int 12 [0xc])) [0 mod+0 S4 A32])
#            (nil))))
    ldl=    $s9,12(,$s3)    # 6 movsi_general/4 [length = 20]

#(insn 18 7 19 (set (reg:DI 8 s8 [144])
#        (mem:DI (plus:DI (reg/f:DI 2 s2)
#                (const_int -276 [0xfffffffffffffeec])) [0 S8 A64])) 57
{*movdi_general} (nil)
#    (nil))
    lds $s8,-276(,$s2)  #movdi case3    # 18    *movdi_general/4    [length = 20]

the first one is OK (loads the only param to calc_mp() in $s9), but the second 
one
(18) loads a value from the first outgoing args slot, where nothing has been 
stored
so far. now, in the initial expansion, 104r.expand, insns between 2 and 18 have
stored a value there, but the optimizer has removed them, mysteriously leaving 
18 in,
which constitutes a read-before-write anyway ... wtf?!

Original comment by jmoc...@gmail.com on 12 Jan 2009 at 3:11

GoogleCodeExporter commented 8 years ago 
even stranger, there are read-before-write insn even in the initial expand 
104r, and
even in the O0 case. indeed, those regs are never referenced later, but: why 
generate
such insns anyway?!

Original comment by jmoc...@gmail.com on 12 Jan 2009 at 3:45

GoogleCodeExporter commented 8 years ago 
fixed with r192.

Original comment by jmoc...@gmail.com on 14 Jan 2009 at 4:20

GoogleCodeExporter commented 8 years ago 
this one seems to have resurfaced with r209.

that is: it failed during the testrun on SX6i, but I can't reproduce it with 
r209 on
an SX8.

fred, if you find a spare moment or two, could you build it with all opt 
levels, test
it on SX6i manually and report here.

Original comment by jmoc...@gmail.com on 29 Jan 2009 at 9:41

GoogleCodeExporter commented 8 years ago

Original comment by jmoc...@gmail.com on 29 Jan 2009 at 9:41

GoogleCodeExporter commented 8 years ago 
With current r211 I do not see it with sx6i :

RUNTESTFLAGS="--target_board=sx6i execute.exp=990106-2.c" make check

                === gcc Summary ===

# of expected passes            12

I would suggest to wait until next testrun to determine if it is fixed ?
or should I revert to r209 and test ? (takes some time to build gcc etc.. )

Original comment by fred.tre...@googlemail.com on 29 Jan 2009 at 9:57

GoogleCodeExporter commented 8 years ago 
it's considered fixed. ;)

Original comment by jmoc...@gmail.com on 29 Jan 2009 at 10:26

GoogleCodeExporter commented 8 years ago 
this one is one persistent mf. here again with r216 ...

Original comment by jmoc...@gmail.com on 3 Feb 2009 at 11:45

GoogleCodeExporter commented 8 years ago 
this one fails *only* when using -msoft-float and only with -O[1|2|s]. ccing 
marko.
need to check which soft-fp routine screws things up.

Original comment by jmoc...@gmail.com on 3 Feb 2009 at 11:47

GoogleCodeExporter commented 8 years ago 
the root of the problem is that we use fp operations + fixing for modulo 
calculation.
somewhere down the road, the CSE step combines a few moves + __floatunsidf() 
call
into a simple (set:DF dst:DF (const_double:DF -1)), which is wrong.

imho, this will be remedied when marko completes the soft- and strict-float 
patch.

Original comment by jmoc...@gmail.com on 3 Feb 2009 at 1:40

GoogleCodeExporter commented 8 years ago 
indeed r220 fixes this.

Original comment by jmoc...@gmail.com on 6 Feb 2009 at 4:25

GoogleCodeExporter commented 8 years ago

Original comment by jmoc...@gmail.com on 6 Feb 2009 at 4:26

GoogleCodeExporter commented 8 years ago 
still fails with strict float in r220 ... sigh ... marko, will you look at it, 
pls?

Original comment by jmoc...@gmail.com on 7 Feb 2009 at 7:03

GoogleCodeExporter commented 8 years ago 
as of r233 gcc implements proper soft/strict/hw float semantics.

Original comment by jmoc...@gmail.com on 17 Feb 2009 at 10:49