public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Kewen.Lin" <linkw@linux.ibm.com>
To: Peter Bergner <bergner@linux.ibm.com>,
	Segher Boessenkool <segher@kernel.crashing.org>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	David Edelsohn <dje.gcc@gmail.com>,
	Michael Meissner <meissner@linux.ibm.com>
Subject: Re: [PATCH] rs6000: Neuter option -mpower{8,9}-vector [PR109987]
Date: Wed, 21 Feb 2024 14:34:15 +0800	[thread overview]
Message-ID: <311c4b81-b2f6-55e9-520b-b659d4eb27f7@linux.ibm.com> (raw)
In-Reply-To: <49f85ead-c409-49b6-a0e6-1a29a39a0b9c@linux.ibm.com>

on 2024/2/21 09:37, Peter Bergner wrote:
> On 2/20/24 3:27 AM, Kewen.Lin wrote:
>> on 2024/2/20 02:45, Segher Boessenkool wrote:
>>> On Tue, Jan 16, 2024 at 10:50:01AM +0800, Kewen.Lin wrote:
>>>> it consists of some aspects:
>>>>   - effective target powerpc_p{8,9}vector_ok are removed
>>>>     and replaced with powerpc_vsx_ok.
>>>
>>> So all such testcases already arrange to have p8 or p9 some other way?
> 
> Shouldn't that be replaced with powerpc_vsx instead of powerpc_vsx_ok?
> That way we know VSX code gen is enabled for the options being used,
> even those in RUNTESTFLAGS.
> 
> I thought we agreed that powerpc_vsx_ok was almost always useless and
> we always want to use powerpc_vsx.  ...or did I miss that we removed
> the old powerpc_vsx_ok and renamed powerpc_vsx to powerpc_vsx_ok?

Yes, I think we all agreed that powerpc_vsx matches more with what we
expect, but I'm hesitating to make such change at this stage because:

  1. if testing on an env without vsx support, the test results on these
     affected test cases may change a lot, as many test cases would
     become unsupported (they pass before with explicit -mvsx before).

  2. teach current powerpc_vsx to make use of current_compiler_flags
     just like some existing practices on has_arch_* may help mitigate
     it as not few test cases already have explicit -mvsx.  But AIUI
     current_complier_flags requires dg-options line comes first before
     the effective target line to make options in dg-options take
     effect, it means we need some work to adjust line order for the
     affected test cases.  On the other hand, some enhancement is needed
     for current_compiler_flags as powerpc_vsx (old powerpc_vsx_ok) isn't
     only used in test case but can be also used in some exp check
     where no expected flags exist.

  3. there may be some other similar effective target checks which we
     want to update as well, it means we need a re-visit on the existing
     effective target checks (rs6000 specific).

  4. powerpc_vsx_ok has been there for a long long time, and -mno-vsx
     is rarely used in RUNTESTFLAGS, this only affects testing, so it
     is not that urgent.

so I'm inclined to work on this in next stage 1.  What do you think?

> 
>>>>   - Some test cases are updated with explicit -mvsx.
>>>>   - Some test cases with those two option mixed are adjusted
>>>>     to keep the test points, like -mpower8-vector
>>>>     -mno-power9-vector are updated with -mdejagnu-cpu=power8
>>>>     -mvsx etc.
>>>
>>> -mcpu=power8 implies -mvsx already.
> 
> Then we can omit the explicit -msx option, correct?  Ie, if the
> user forces -mno-vsx in RUNTESTFLAGS, then we'll just skip the
> test case as UNSUPPORTED rather than trying to compile some
> vsx test case with vsx disabled via the options.

Yes, we can strip any -mvsx then, but if we want the test case
to be tested when it's able to, we can still append an extra
-mvsx.  Even if -mno-vsx is specified but if the option order
makes it like "-mno-vsx... -mvsx", powerpc_vsx is supported
so that the test case can be still tested well with -mvsx
enabled, while if the order is like "-mvsx ... -mno-vsx",
powerpc_vsx fails and it becomes unsupported.

BR,
Kewen


      reply	other threads:[~2024-02-21  6:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-16  2:50 Kewen.Lin
2024-02-19 18:45 ` Segher Boessenkool
2024-02-20  9:27   ` Kewen.Lin
2024-02-20 11:19     ` Segher Boessenkool
2024-02-21  6:05       ` Kewen.Lin
2024-02-21  1:37     ` Peter Bergner
2024-02-21  6:34       ` Kewen.Lin [this message]

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=311c4b81-b2f6-55e9-520b-b659d4eb27f7@linux.ibm.com \
    --to=linkw@linux.ibm.com \
    --cc=bergner@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=meissner@linux.ibm.com \
    --cc=segher@kernel.crashing.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).