Re: [PATCH] rs6000: Rework option -mpowerpc64 handling [PR106680]

[Segher Boessenkool <segher@kernel.crashing.org>]

On Mon, Oct 10, 2022 at 10:15:58AM +0800, Kewen.Lin wrote:
> on 2022/10/4 05:15, Segher Boessenkool wrote:
> > Right.  If If mpowerpc64 is enabled while OS_MISSING_POWERPC64, warn for
> > that; 
> 
> Currently if option powerpc64 is enabled explicitly while OS_MISSING_POWERPC64,
> there is no warning.  One typical case is -m32 compilation on ppc64.  I made
> a patch to warn for this case as you suggested (btw, this change can be taken
> separately from this rework), it caused some test cases to fail as below:

"Explicitly" means the user says "-m32 -mpowerpc64".

I wonder what "on powerpc64" means in what you say, and why that would
matter?

> gcc.dg/vect/vect-82_64.c
> gcc.dg/vect/vect-83_64.c
> gcc.target/powerpc/bswap64-4.c
> gcc.target/powerpc/ppc64-double-1.c
> gcc.target/powerpc/pr106680-4.c 
> gcc.target/powerpc/rs6000-fpint-2.c
> 
> It's fine to fix them with one additional option "-w" to disable the warning.
> But IIUC one concern is that if we want to test with "--target_board=unix'{-m32,
> -m32/-mpowerpc64}'", the latter combination will always have this warning,
> with one extra "-w" (that is -m32/-mpowerpc64/-w) can make some cases which
> aim to check warning msg ineffective.  So maybe we want to re-consider it
> (like just leaving it as before)?

There will always be false positives (and negatives!) if you put any
warning options in RUNTESTFLAGS.  -w is merely louder than most :-)

But leave this as further improvement.  Maybe put in a comment.

> > and if mpowerpc64 was only implicit, disable it as well (and say
> > we did!)
> 
> But on ppc64 linux, for -m32 compilation mpowerpc64 is implicitly enabled
> since it's with bi-arch supported, I made a patch to disable it as well as
> warn it, it can't be bootstrapped since it warned for -m32 build (-Werror)
> and failed.  So I refined it to something like:
> 
> +          /* With RS6000_BI_ARCH defined (bi-architecture (32/64) supported),
> +             TARGET_DEFAULT has bit MASK_POWERPC64 on by default, to keep the
> +             behavior consistent (like: no warnings for -m32 on ppc64), we
> +             just sliently disable it.  Otherwise, disable it and warn.  */
> +          rs6000_isa_flags &= ~OPTION_MASK_POWERPC64;
> +#ifndef RS6000_BI_ARCH
> +          warning (0, "powerpc64 is unexpected to be enabled on the "
> +                      "current OS");
> +#endif

It has nothing to do with biarch.  Let's just not warn if it is so much
work to do it correctly.  We never did before, and no one complained,
how bad can it be :-)


Segher