All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@mips.com>
To: James Hogan <james.hogan@mips.com>
Cc: <linux-mips@linux-mips.org>, James Hogan <jhogan@kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Paul Burton <paul.burton@imgtec.com>, <stable@vger.kernel.org>
Subject: Re: [PATCH] MIPS: Fix odd fp register warnings with MIPS64r2
Date: Thu, 23 Nov 2017 14:42:35 +0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1711231420400.4290@tp.orcam.me.uk> (raw)
In-Reply-To: <20171110205519.25884-1-james.hogan@mips.com>

On Fri, 10 Nov 2017, James Hogan wrote:

> Building 32-bit MIPS64r2 kernels produces warnings like the following
> on certain toolchains (such as GNU assembler 2.24.90, but not GNU
> assembler 2.28.51) since commit 22b8ba765a72 ("MIPS: Fix MIPS64 FP
> save/restore on 32-bit kernels"), due to the exposure of fpu_save_16odd
> from fpu_save_double and fpu_restore_16odd from fpu_restore_double:
> 
> arch/mips/kernel/r4k_fpu.S:47: Warning: float register should be even, was 1
> ...
> arch/mips/kernel/r4k_fpu.S:59: Warning: float register should be even, was 1
> ...

 Hmm, versions 2.24.90 and 2.28.51 are otherwise unindentified development 
snapshots; I think it would be slightly more appropriate if you referred 
to actual release versions, such as 2.25 and 2.29 respectively (if they 
indeed expose the same symptoms), especially as people other than 
toolchain developers and testers are generally expected not to use 
development snapshots.

 Also I find it suspicious that you say that the message has since 
vanished, as I can clearly reproduce it with current head (2.29.51):

$ cat oddfpr.s
	.module mips64r2
foo:
	ldc1	$f1, 0($2)
$ as -o oddfpr.o oddfpr.s
oddfpr.s: Assembler messages:
oddfpr.s:3: Warning: float register should be even, was 1
$ 

Can you send me a .s file produced from r4k_fpu.S along with compiler 
flags used?

> This appears to be because .set mips64r2 does not change the FPU ABI to
> 64-bit when -march=mips64r2 (or e.g. -march=xlp) is provided on the
> command line on that toolchain, from the default FPU ABI of 32-bit due
> to the -mabi=32. This makes access to the odd FPU registers invalid.

 Correct, the purpose of `.set arch=mips64r2', which is the canonical form 
`.set mips64r2' is equivalent to, and indeed any `.set arch=...' 
pseudo-op, is only to change the set of instructions accepted within the 
limits determined by the ABI chosen with `-mabi=...' (or its equivalent 
short forms) and not the ABI itself.

 The patch itself LGTM.

Reviewed-by: Maciej W. Rozycki <macro@mips.com>

  Maciej

WARNING: multiple messages have this Message-ID (diff)
From: "Maciej W. Rozycki" <macro@mips.com>
To: James Hogan <james.hogan@mips.com>
Cc: linux-mips@linux-mips.org, James Hogan <jhogan@kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Paul Burton <paul.burton@imgtec.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH] MIPS: Fix odd fp register warnings with MIPS64r2
Date: Thu, 23 Nov 2017 14:42:35 +0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1711231420400.4290@tp.orcam.me.uk> (raw)
Message-ID: <20171123144235.NarRPzEhrsHYKNFJo5QsZzY7prg0nxh9wxDMT4j9gDo@z> (raw)
In-Reply-To: <20171110205519.25884-1-james.hogan@mips.com>

On Fri, 10 Nov 2017, James Hogan wrote:

> Building 32-bit MIPS64r2 kernels produces warnings like the following
> on certain toolchains (such as GNU assembler 2.24.90, but not GNU
> assembler 2.28.51) since commit 22b8ba765a72 ("MIPS: Fix MIPS64 FP
> save/restore on 32-bit kernels"), due to the exposure of fpu_save_16odd
> from fpu_save_double and fpu_restore_16odd from fpu_restore_double:
> 
> arch/mips/kernel/r4k_fpu.S:47: Warning: float register should be even, was 1
> ...
> arch/mips/kernel/r4k_fpu.S:59: Warning: float register should be even, was 1
> ...

 Hmm, versions 2.24.90 and 2.28.51 are otherwise unindentified development 
snapshots; I think it would be slightly more appropriate if you referred 
to actual release versions, such as 2.25 and 2.29 respectively (if they 
indeed expose the same symptoms), especially as people other than 
toolchain developers and testers are generally expected not to use 
development snapshots.

 Also I find it suspicious that you say that the message has since 
vanished, as I can clearly reproduce it with current head (2.29.51):

$ cat oddfpr.s
	.module mips64r2
foo:
	ldc1	$f1, 0($2)
$ as -o oddfpr.o oddfpr.s
oddfpr.s: Assembler messages:
oddfpr.s:3: Warning: float register should be even, was 1
$ 

Can you send me a .s file produced from r4k_fpu.S along with compiler 
flags used?

> This appears to be because .set mips64r2 does not change the FPU ABI to
> 64-bit when -march=mips64r2 (or e.g. -march=xlp) is provided on the
> command line on that toolchain, from the default FPU ABI of 32-bit due
> to the -mabi=32. This makes access to the odd FPU registers invalid.

 Correct, the purpose of `.set arch=mips64r2', which is the canonical form 
`.set mips64r2' is equivalent to, and indeed any `.set arch=...' 
pseudo-op, is only to change the set of instructions accepted within the 
limits determined by the ABI chosen with `-mabi=...' (or its equivalent 
short forms) and not the ABI itself.

 The patch itself LGTM.

Reviewed-by: Maciej W. Rozycki <macro@mips.com>

  Maciej

  reply	other threads:[~2017-11-23 14:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-10 20:55 [PATCH] MIPS: Fix odd fp register warnings with MIPS64r2 James Hogan
2017-11-10 20:55 ` James Hogan
2017-11-23 14:42 ` Maciej W. Rozycki [this message]
2017-11-23 14:42   ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1711231420400.4290@tp.orcam.me.uk \
    --to=macro@mips.com \
    --cc=james.hogan@mips.com \
    --cc=jhogan@kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=paul.burton@imgtec.com \
    --cc=ralf@linux-mips.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.