On Thu, Feb 06, 2014 at 12:09:00PM +1000, Stephen N Chivers wrote:
> I have a MPC8548e based board and an application that makes
> extensive use of floating point including numerous calls to cos.
> In the same program there is the use of an sqlite database.
>
> The kernel is derived from 2.6.31 and is compiled with math emulation.
>
> At some point after the reading of the SQLITE database, the
> return result from cos goes from an in range value to an out
> of range value.
>
> This is as a result of the FP rounding mode mutating from "round to
> nearest"
> to "round toward zero".
>
> The cos in the glibc version being used is known to be sensitive to
> rounding
> direction and Joseph Myers has previously fixed glibc.
>
> The failure does not occur on a machine that has a hardware floating
> point unit (a MPC7410 processor).
>
> I have traced the mutation to the following series of instructions:
>
> mffs f0
> mtfsb1 4*cr7+so
> mtfsb0 4*cr7+eq
> fadd f13,f1,f2
> mtfsf 1, f0
>
> The instructions are part of the stream emitted by gcc for the conversion
> of a 128 bit floating point value into an integer in the sqlite database
> read.
>
> Immediately before the execution of the mffs instruction the "rounding
> mode" is "round to nearest".
>
> On the MPC8548 board, the execution of the mtfsf instruction does not
> restore the rounding mode to "round to nearest".
>
> I believe that the mask computation in mtfsf.c is incorrect and is
> reversed.
>
> In the latest version of the file (linux-3.14-rc1), the mask is computed
> by:
>
> mask = 0;
> if (FM & (1 << 0))
> mask |= 0x90000000;
> if (FM & (1 << 1))
> mask |= 0x0f000000;
> if (FM & (1 << 2))
> mask |= 0x00f00000;
> if (FM & (1 << 3))
> mask |= 0x000f0000;
> if (FM & (1 << 4))
> mask |= 0x0000f000;
> if (FM & (1 << 5))
> mask |= 0x00000f00;
> if (FM & (1 << 6))
> mask |= 0x000000f0;
> if (FM & (1 << 7))
> mask |= 0x0000000f;
>
> I think it should be:
>
> mask = 0;
> if (FM & (1 << 0))
> mask |= 0x0000000f;
> if (FM & (1 << 1))
> mask |= 0x000000f0;
> if (FM & (1 << 2))
> mask |= 0x00000f00;
> if (FM & (1 << 3))
> mask |= 0x0000f000;
> if (FM & (1 << 4))
> mask |= 0x000f0000;
> if (FM & (1 << 5))
> mask |= 0x00f00000;
> if (FM & (1 << 6))
> mask |= 0x0f000000;
> if (FM & (1 << 7))
> mask |= 0x90000000;
>
> With the above mask computation I get consistent results for both the
> MPC8548
> and MPC7410 boards.
>
> Am I missing something subtle?
No I think you are correct. This said, this code may probably be optimized
to eliminate a lot of the conditional branches. I think that:
mask = (FM & 1);
mask |= (FM << 3) & 0x10;
mask |= (FM << 6) & 0x100;
mask |= (FM << 9) & 0x1000;
mask |= (FM << 12) & 0x10000;
mask |= (FM << 15) & 0x100000;
mask |= (FM << 18) & 0x1000000;
mask |= (FM << 21) & 0x10000000;
mask *= 15;
should do the job, in less code space and without a single branch.
Each one of the "mask |=" lines should be translated into an
rlwinm instruction followed by an "or". Actually it should be possible
to transform each of these lines into a single "rlwimi" instruction
but I don't know how to coerce gcc to reach this level of optimization.
Another way of optomizing this could be:
mask = (FM & 0x0f) | ((FM << 12) & 0x000f0000);
mask = (mask & 0x00030003) | ((mask << 6) & 0x03030303);
mask = (mask & 0x01010101) | ((mask << 3) & 0x10101010);
mask *= 15;
It's not easy to see which of the solutions is faster, the second one
needs to create quite a few constants, but its dependency length is
lower. It is very likely that the first solution is faster in cache-cold
case and the second in cache-hot.
Regardless, the original code is rather naïve, larger and slower in all cases,
with timing variation depending on branch mispredictions.
Regards,
Gabriel
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
