diff mbox

Update Linux kernel to current glibc soft-fp

Message ID mf0mpn$1j8$1@ger.gmane.org
State Superseded
Headers show

Commit Message

Stefan Liebler March 26, 2015, 10:24 a.m. UTC
Hi Joseph,

I have news for s390.
The removal of files in arch/s390/math-emu can be found in linux-next:
(http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=723547a1a0f83ac30039feea11a3723caafd2361)

Search after:
commit 723547a1a0f83ac30039feea11a3723caafd2361
Author: Heiko Carstens <heiko.carstens@de.ibm.com>
Date:   Thu Feb 12 13:08:27 2015 +0100

     s390: remove 31 bit support

     Remove the 31 bit support in order to reduce maintenance cost and
     effectively remove dead code. Since a couple of years there is no
     distribution left that comes with a 31 bit kernel.

     The 31 bit kernel also has been broken since more than a year before
     anybody noticed. In addition I added a removal warning to the kernel
     shown at ipl for 5 minutes: a960062e5826 ("s390: add 31 bit warning
     message") which let everybody know about the plan to remove 31 bit
     code. We didn't get any response.

     Given that the last 31 bit only machine was introduced in 1999 let's
     remove the code.
     Anybody with 31 bit user space code can still use the compat mode.

     Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
     Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>


Nevertheless some macros FP_* are used
in arch/s390/kernel/sysinfo.c in s390_adjust_jiffies()-method.
The result can be seen in /proc/cpuinfo: "bogomips per cpu: xyz"



After applying your patch, i got the following build-errors:

   CC      arch/s390/kernel/sysinfo.o
In file included from 
/media/5ec1/kernelDir/src/include/math-emu/soft-fp.h:321:0,
                  from 
/media/5ec1/kernelDir/src/arch/s390/kernel/sysinfo.c:22:
/media/5ec1/kernelDir/src/arch/s390/kernel/sysinfo.c: In function 
‘s390_adjust_jiffies’:
/media/5ec1/kernelDir/src/include/math-emu/op-common.h:1800:10: error: 
lvalue required as left operand of assignment
       (r) = -(rtype) (r);      \
           ^
/media/5ec1/kernelDir/src/include/math-emu/single.h:188:37: note: in 
expansion of macro ‘_FP_FROM_INT’
  #define FP_FROM_INT_S(X, r, rs, rt) _FP_FROM_INT (S, 1, X, (r), (rs), rt)
                                      ^
/media/5ec1/kernelDir/src/arch/s390/kernel/sysinfo.c:440:4: note: in 
expansion of macro ‘FP_FROM_INT_S’
     FP_FROM_INT_S(SB, (long) info->capability, 64, long);
     ^
In file included from 
/media/5ec1/kernelDir/src/include/math-emu/soft-fp.h:40:0,
                  from 
/media/5ec1/kernelDir/src/arch/s390/kernel/sysinfo.c:22:
/media/5ec1/kernelDir/src/arch/s390/include/asm/sfp-machine.h:138:22: 
error: ‘mode’ undeclared (first use in this function)
  #define FP_ROUNDMODE mode
                       ^
/media/5ec1/kernelDir/src/include/math-emu/op-common.h:132:11: note: in 
expansion of macro ‘FP_ROUNDMODE’
        if (FP_ROUNDMODE == FP_RND_NEAREST  \
            ^
/media/5ec1/kernelDir/src/include/math-emu/op-common.h:1828:8: note: in 
expansion of macro ‘_FP_OVERFLOW_SEMIRAW’
         _FP_OVERFLOW_SEMIRAW (fs, wc, X);    \
         ^
/media/5ec1/kernelDir/src/include/math-emu/single.h:188:37: note: in 
expansion of macro ‘_FP_FROM_INT’
  #define FP_FROM_INT_S(X, r, rs, rt) _FP_FROM_INT (S, 1, X, (r), (rs), rt)
                                      ^
/media/5ec1/kernelDir/src/arch/s390/kernel/sysinfo.c:440:4: note: in 
expansion of macro ‘FP_FROM_INT_S’
     FP_FROM_INT_S(SB, (long) info->capability, 64, long);
     ^
/media/5ec1/kernelDir/src/arch/s390/include/asm/sfp-machine.h:138:22: 
note: each undeclared identifier is reported only once for each function 
it appears in
  #define FP_ROUNDMODE mode
                       ^
/media/5ec1/kernelDir/src/include/math-emu/op-common.h:132:11: note: in 
expansion of macro ‘FP_ROUNDMODE’
        if (FP_ROUNDMODE == FP_RND_NEAREST  \
            ^
/media/5ec1/kernelDir/src/include/math-emu/op-common.h:1828:8: note: in 
expansion of macro ‘_FP_OVERFLOW_SEMIRAW’
         _FP_OVERFLOW_SEMIRAW (fs, wc, X);    \
         ^
/media/5ec1/kernelDir/src/include/math-emu/single.h:188:37: note: in 
expansion of macro ‘_FP_FROM_INT’
  #define FP_FROM_INT_S(X, r, rs, rt) _FP_FROM_INT (S, 1, X, (r), (rs), rt)
                                      ^
/media/5ec1/kernelDir/src/arch/s390/kernel/sysinfo.c:440:4: note: in 
expansion of macro ‘FP_FROM_INT_S’
     FP_FROM_INT_S(SB, (long) info->capability, 64, long);
     ^
make[4]: *** [arch/s390/kernel/sysinfo.o] Error 1




According to "error: lvalue required as left operand of assignment":
Why did you write back r in include/math-emu/op-common.h:
#define _FP_FROM_INT(fs, wc, X, r, rsize, rtype):
	  if ((X##_s = ((r) < 0)))
	    (r) = -(rtype) (r);
	  _FP_FROM_INT_ur = (rtype) (r);


According to "error: ‘mode’ undeclared (first use in this function)":
We´ve added the variable like it is done in arcg/s390/math-emu/math.c.

With the following patch, the kernel builds and is starting,
but /proc/cpuinfo shows "bogomips per cpu: 0" instead of the previous 
number:





Please help.

Bye Stefan



On 03/23/2015 06:04 PM, Joseph Myers wrote:
> Here is an updated version of the Linux kernel patch
> <https://sourceware.org/ml/libc-alpha/2015-02/msg00107.html>, using
> current soft-fp code so that the code proposed for the kernel is
> *identical* to the code in glibc (and incorporating fixes for the abort
> issue).
>
> Testing is needed for alpha, sh, sparc (both 32-bit and 64-bit).  As I
> haven't yet seen the patch referred to in
> <https://sourceware.org/ml/libc-alpha/2015-02/msg00464.html>, the s390
> changes are still in this patch version.
>
> The previous description of appropriate testing applies (and I've done
> this testing for powerpc):
>
> The minimum testing is:
>
> * Apply to a current Linux git tree, and build in a configuration in
>    which the affected code for your architecture is enabled.  In the
>    sparc case that means covering both math_32.c and math_64.c.
>
> * If there are build problems from this code (including warnings),
>    either fix them and send me the patch (on top of this one) or let me
>    know the problems so I can fix them.
>
> However, it's quite possible some problems will not show up that way,
> so it's also useful if possible to do the following:
>
> * Boot the new kernel on a system (physical or virtual) where the
>    emulation code will be used, and run the glibc math/ tests (glibc
>    built for hard-float) on that system, making sure the results are no
>    worse than for an old kernel (that is, that no failures appear that
>    seem likely to be related to the changes; if your existing baseline
>    has some libm test failures, that means looking in more detail at
>    *which* individual tests in test-float, test-double, test-ldouble
>    are failing before and after the kernel patch, to make sure it isn't
>    introducing such failures).  If you see such failures, again, either
>    debug and fix them or report them to me (though instrumenting the
>    relevant functions to track down which operation's results are
>    changing may help a lot in finding where the problem it).
>
> * Do whatever review of the changes for your architecture seems
>    appropriate.
>

Comments

Joseph Myers April 2, 2015, 5:44 p.m. UTC | #1
On Thu, 26 Mar 2015, Stefan Liebler wrote:

> According to "error: lvalue required as left operand of assignment":
> Why did you write back r in include/math-emu/op-common.h:
> #define _FP_FROM_INT(fs, wc, X, r, rsize, rtype):
> 	  if ((X##_s = ((r) < 0)))
> 	    (r) = -(rtype) (r);
> 	  _FP_FROM_INT_ur = (rtype) (r);

Because the _FP_FROM_INT interface requires an argument of the correct 
signedness, but needs to work internally on an unsigned value, so starts 
by negating a signed argument.

> @@ -435,11 +436,12 @@ void s390_adjust_jiffies(void)
>  		 * by the cpu capability number. Yes, that means a floating
>  		 * point division .. math-emu here we come :-)
>  		 */
> -		FP_UNPACK_SP(SA, &fmil);
> -		if ((info->capability >> 23) == 0)
> -			FP_FROM_INT_S(SB, (long) info->capability, 64, long);
> -		else
> -			FP_UNPACK_SP(SB, &info->capability);
> +		FP_UNPACK_SEMIRAW_SP(SA, &fmil);
> +		if ((info->capability >> 23) == 0) {
> +			unsigned long r = info->capability;
> +			FP_FROM_INT_S(SB, r, 64, unsigned long);
> +		} else
> +			FP_UNPACK_SEMIRAW_SP(SB, &info->capability);
>  		FP_DIV_S(SR, SA, SB);
>  		FP_TO_INT_S(capability, SR, 32, 0);

Division uses cooked inputs and outputs.  FP_TO_INT uses raw inputs.  
FP_FROM_INT uses raw outputs.

So for unpacking SA you should continue to use FP_UNPACK_SP, as the result 
goes straight into division.  For unpacking SB, it seems appropriate to 
use FP_UNPACK_RAW_SP.  Then, after either unpacking or FP_FROM_INT_S, you 
have a raw value in SB, and can use _FP_UNPACK_CANONICAL to produce a 
cooked value from it that can be used as an input to the division.

As for the integer argument to FP_FROM_INT_S, the existing code treats it 
as signed long, so the same semantics would be preserved by making the 
temporary variable of that type (however, you still need to pass "unsigned 
long" as the last argument to FP_FROM_INT_S, as it expects the type name 
passed to be the name of an unsigned type).
diff mbox

Patch

diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
index 99babea..971d13b 100644
--- a/arch/s390/kernel/sysinfo.c
+++ b/arch/s390/kernel/sysinfo.c
@@ -418,6 +418,7 @@  void s390_adjust_jiffies(void)
  	FP_DECL_S(SA); FP_DECL_S(SB); FP_DECL_S(SR);
  	FP_DECL_EX;
  	unsigned int capability;
+	int mode = 0;

  	info = (void *) get_zeroed_page(GFP_KERNEL);
  	if (!info)
@@ -435,11 +436,12 @@  void s390_adjust_jiffies(void)
  		 * by the cpu capability number. Yes, that means a floating
  		 * point division .. math-emu here we come :-)
  		 */
-		FP_UNPACK_SP(SA, &fmil);
-		if ((info->capability >> 23) == 0)
-			FP_FROM_INT_S(SB, (long) info->capability, 64, long);
-		else
-			FP_UNPACK_SP(SB, &info->capability);
+		FP_UNPACK_SEMIRAW_SP(SA, &fmil);
+		if ((info->capability >> 23) == 0) {
+			unsigned long r = info->capability;
+			FP_FROM_INT_S(SB, r, 64, unsigned long);
+		} else
+			FP_UNPACK_SEMIRAW_SP(SB, &info->capability);
  		FP_DIV_S(SR, SA, SB);
  		FP_TO_INT_S(capability, SR, 32, 0);
  	} else