Message-ID: <alpine.DEB.2.00.1604220127530.21846@tp.orcam.me.uk>
Date:	Fri, 22 Apr 2016 02:00:47 +0100
From:	"Maciej W. Rozycki" <macro@...tec.com>
To:	Matthew Fortune <Matthew.Fortune@...tec.com>
CC:	Ralf Baechle <ralf@...ux-mips.org>,
	"Maciej W. Rozycki" <macro@...ux-mips.org>,
	kbuild test robot <fengguang.wu@...el.com>,
	Paul Burton <Paul.Burton@...tec.com>,
	"kbuild-all@...org" <kbuild-all@...org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: {standard input}:136: Error: number (0x9000000080000000) larger
 than 32 bits

On Tue, 19 Apr 2016, Matthew Fortune wrote:

> >  I have a fix in the works and to have it integrated upstream I just
> > need to accompany it with suitable cases -- like the fragment above --
> > for the GAS testsuite.  I'll send an update when it's ready.
> 
> I do not think the change in behaviour was intentional. I think it came
> about because I separated the handling of an explicit .set mips* directive
> from the implicit setting of gp/fp. I needed to ensure that gp/fp was
> only updated when there was a .set mips* directive but missed the case
> where you have an explicit directive that matches the current mips*
> setting.

 I had thought it might have been intentional before I realised the 
problem also affected ISA restoration, at which point I concluded it had 
been simply an oversight.

 While looking at it I've noticed we support somewhat questionable 
settings with the related `.module' directive, such as:

	.module	at=$k0
	.module	nomove

They have no command-line counterparts, so there's really no difference 
between having a `.module' or a corresponding `.set' directive at the top 
of a source file.  There's no `.module' support for the similar `nomacro' 
or `noreorder' settings though.  Was there a hidden motivation that you 
had to make such an arrangement?

> The conditional nature of updating 'fp' is however very much intentional
> in that if a user enters 'fpxx' mode where fp==0 then the implicit update
> of fp does not happen. This is OK as this behaviour only triggers when
> using the newly added fpxx feature anyway.

 Sure, my only concern has been the inconsistency between the case where 
the ISA is actually switched and one where it remains the same after a 
`.set' pseudo-op.

> I am generally not in favour of these implicit side-effect behaviours
> as they are only useful in niche cases and are somewhat non-intuitive but
> we are stuck with them because of history. It would be possible to write
> code that does not need them by explicitly using the .set gp or .set fp
> options. There is little we can do about that given historic precedent
> though. Instead code that does not want the implicit behaviour has to
> explicitly do things like:
> 
> .set mips3
> .set gp=default
> .set fp=default

 Well, historically we had very little control and old code adapted to it.  
Even the `.set gp=' and `.set fp=' directives are relatively recent in the 
history of the MIPS port.  So I agree -- had we had control we have today 
back when the MIPS port started, this would have been better handled with 
these more finegrained knobs.  Before NewABI support was added even using 
e.g. `-mips3' on the command line made full 64-bit registers available.

 I have fixed the bug upstream now:

commit 22522f880a8e17a17c4f195796ec89caece7652b
Author: Maciej W. Rozycki <macro@...tec.com>
Date:   Fri Apr 22 01:04:52 2016 +0100

    MIPS/GAS: Fix an ISA override not lifting ABI restrictions

and included suitable test cases with the fix.  The lack of testsuite 
coverage here is what let this bug remain unnoticed.

  Maciej

Powered by blists - more mailing lists