Messages in this thread | | | From | Nick Desaulniers <> | Date | Tue, 23 Jun 2020 13:58:15 -0700 | Subject | Re: [PATCH 2/2] selftests/fpu: Add an FPU selftest |
| |
On Tue, Jun 23, 2020 at 12:13 PM Borislav Petkov <bp@alien8.de> wrote: > > On Tue, Jun 23, 2020 at 12:07:13PM -0700, Nick Desaulniers wrote: > > You already have a conditional below for CC_IS_GCC; just add an else > > and unconditionally add -msse2. You *should* use -msse2 for GCC 7.1+ > > IMO. > > Why if one can write it more compact with cc-option? > > FPU_CFLAGS += $(call cc-option,-msse2,)
This will always be true though, right? For both compilers. So why test for it via cc-option when you can just do:
FPU_CFLAGS += -msse2
Though we might only want -msse2 if CC_IS_CLAG...I don't remember now if GCC will select SSE instructions that require 16B operands at -msse2, which will be problematic for the -mpreferred-stack-boundary=4 case.
My point was more so about avoiding needless cc-option checks when they're tautological.
> > I recommend a version check for GCC < 7.1, or simply disabling the > > self test if the version of GCC used is older than 7.1. > > See Andy's suggestion upthread.
Thread for other travelers: https://lore.kernel.org/lkml/CALCETrXXzt8WZMs3dsReCJ5wdF3zhxFmUtGnmdCgV7_exFUKKQ@mail.gmail.com/
When Andy says "consider dropping the problematic GCC version" I wonder if it was meant *just for this selftest* as I suggested, or outright (which is untenable IMO, as it's a large jump to GCC 7.1+).
> And I agree too that using cc-option is better than simply tying it to a > compiler version.
I agree that will differentiate better than a version check; but it's still dangerous IMO to mix and match stack alignments.
> Who knows what compiler has what backported. In such > cases a version number means nothing.
I'm not sure I agree, but I'll take feature detection any day over version detection.
> > > ^ looks familiar ;) > > It has been pasted around the kernel, I came to realize today. :-)
Guilty, your honor. :P
Maybe the feature test should be copy+pasta'd to those other places in the kernel, rather than the version check? -- Thanks, ~Nick Desaulniers
| |