Re: [PATCH v3] MIPS: Implement ieee754 NAN2008 emulation mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 15 Jul 2024, Jiaxun Yang wrote:

> >  I don't know what prctl(2) has to do with this.  If you don't implement 
> > this part, then your change will cause Linux to behave inconsistently and 
> > therefore I'll have to NAK it.
> 
> I think your concern was regarding user space application needs to set NaN2008 bits
> at runtime?

 Nope, following the objective of your change: the EF_MIPS_NAN2008 ELF 
file header flag instructs the kernel to choose between hardware and 
emulated hard float and that's not supposed to change later on throughout 
the life of the program, because it's not something the program can do 
itself, because writes to FCSR.NAN2008 are ignored by hardware.  And it's 
not a functional regression, because flipping FCSR.NAN2008 isn't allowed 
by hardware concerned anyway, we just want to have it consistent including 
the debugger interface.

> >  It's not much to do anyway, as I have prepared `ptrace_setfcr31' already 
> > to handle masking correctly, so all you have to do is to set the mask as 
> > required for the right thing to happen.  I shouldn't have needed to point 
> > you at it though, as that code is easy to find.
> 
> I think I got your point, will try to implement it.

 Thank you.

> >  This doesn't matter either, as your change only addresses the case where 
> > FCSR.NAN2008 isn't writable anyway, which is the sole reason you want to 
> > switch between native hard float support and emulation, doesn't it?
> >
> >  In fact where FCSR.NAN2008 is writable your new mode has to be equivalent
> > to "ieee754=strict", because then there is no need to trigger emulation 
> > for either NaN mode.  Please do verify that this is the case.
> 
> This had been verified with perf math-emu counters to ensure no unnecessary emulation
> is triggered.

 Thanks.

> >  That doesn't matter for us here (and I have a bad suspicion anyway), but 
> > the Debian team is of course free to do what they want here, the GNU GPL 
> > applies.
> 
> We care about our downstream users, don't we?

 There is a balance for us to keep.  Requests made have to be reasonable
and code contributed has to be architected well and meet quality criteria.  
Every change carries its associated cost and especially with the limited 
manpower available we can't afford having a technical debt created.  Any 
unclean piece of code accepted will strike us back sooner or later.

> They asked me to help and that was my solution. I sincerely want to get this change upstreamed
> to cover some downstream use cases.

 If it's your own genuine from-scratch implementation, then I have more 
faith in it.

> I don't know what theory do you have here, but that's all stories behind.

 I have seen odd requests and code changes stemming from embarassing lack 
of understanding how things work with the MIPS architecture.

> >  And also they can always use the "nofpu" kernel parameter to run their 
> > verification.  I used it for mine back at ImgTec before 2008 NaN hardware 
> > was available, also to verify emulation, which I wrote too.  Perhaps that 
> > is also the right solution for Debian actually?
> 
> I'll suggest to them, thanks.

 I note that it's been like this since 2015 and it has been documented:

	ieee754=	[MIPS] Select IEEE Std 754 conformance mode
			Format: { strict | legacy | 2008 | relaxed }
			Default: strict
[...]
			The FPU emulator is always able to support both NaN
			encodings, so if no FPU hardware is present or it has
			been disabled with 'nofpu', then the settings of
			'legacy' and '2008' strap the emulator accordingly,
			'relaxed' straps the emulator for both legacy-NaN and
			2008-NaN, whereas 'strict' enables legacy-NaN only on
			legacy processors and both NaN encodings on MIPS32 or
			MIPS64 CPUs.

(see the part following the last comma in particular).  It usually makes 
sense to read documentation and I'd expect MIPS Debian port experts to do 
it from time to time.

  Maciej




[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux