public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Kewen.Lin" <linkw@linux.ibm.com>
To: guojiufu <guojiufu@linux.ibm.com>
Cc: segher@kernel.crashing.org, dje.gcc@gmail.com, linkw@gcc.gnu.org,
	meissner@linux.ibm.com, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] testsuite: update requires for powerpc/float128-cmp2-runnable.c
Date: Tue, 11 Apr 2023 17:40:09 +0800	[thread overview]
Message-ID: <13ec00da-587b-847d-c26b-98cf463f21ac@linux.ibm.com> (raw)
In-Reply-To: <71ed6f665ae2ed9678d8dc4ec0f620ce@linux.ibm.com>

Hi Jeff,

on 2023/4/11 17:14, guojiufu wrote:
> Hi Kewen,
> 
> Thanks a lot for your very helpful comments!
> 
> On 2023-04-10 17:26, Kewen.Lin wrote:
>> Hi Jeff,
>>
>> on 2023/4/10 10:09, Jiufu Guo via Gcc-patches wrote:
>>> Hi,
>>>
>>> In this test case (float128-cmp2-runnable.c), the instruction
>>> xscmpexpqp is used to support a few builtins e.g.
>>> __builtin_vsx_scalar_cmp_exp_qp_eq on _Float128.
>>> This instruction handles the whole 128bits of the vector, and
>>> it is guarded by [ieee128-hw].
>>
>> The instruction xscmpexpqp is guarded with TARGET_P9_VECTOR,
>>
>> (define_insn "*xscmpexpqp"
>>   [(set (match_operand:CCFP 0 "cc_reg_operand" "=y")
>>     (compare:CCFP
>>      (unspec:IEEE128 [(match_operand:IEEE128 1 "altivec_register_operand" "v")
>>                   (match_operand:IEEE128 2 "altivec_register_operand" "v")]
>>       UNSPEC_VSX_SCMPEXPQP)
>>      (match_operand:SI 3 "zero_constant" "j")))]
>>   "TARGET_P9_VECTOR"
>>   "xscmpexpqp %0,%1,%2"
>>   [(set_attr "type" "fpcompare")])
>>
>> [ieee128-hw] is used for guarding those bifs, so the above
>> statement doesn't quite match the fact.
>>
> 
> Agree, I'm wondering if P9_VECTOR is perfect here, even if it indicates the ISA
> which contains xscmpexpqp. Let me have more checks.
> 
>> PR108758 said this case doesn't fail with gcc-10 and gcc-11,
>> I wonder why it changes from gcc-12?  The above define_insn
>> shows the underlying insns for these bifs just requires the
>> condition power9-vector.  Could you have a further check?
>> Thanks.
> 
> Thanks for raising this concern.
> The behavior to check about bif on FLOAT128_HW and emit an error message for
> requirements on quad-precision is added in gcc12. This is why gcc12 fails to
> compile the case on -m32.
> 
> Before gcc12, altivec_resolve_overloaded_builtin will return the overloaded
> result directly, and does not check more about the result function.

Thanks for checking, I wonder which commit caused this behavior change and what's
the underlying justification?  I know there is one new bif handling framework
introduced in gcc12, not sure the checking condition was changed together or by
a standalone commit.  Anyway, apparently the conditions for the support of these
bifs are different on gcc-11 and gcc-12, I wonder why it changed.  As mentioned
above, PR108758's c#1 said this case (bifs) work well on gcc-11, I suspected the
condition change was an overkill, that's why I asked.

BR,
Kewen

> 
>>
>> btw, please add a PR marker for PR108758.
> 
> Sure,  thanks for catching this!
> 
> 
> BR,
> Jeff (Jiufu)
> 
>>
>> BR,
>> Kewen
>>
>>> So, we may update the testcase to require ppc_float128_hw.
>>>
>>> Tested on ppc64 both BE and LE.
>>> Is this ok for trunk?
>>>
>>> BR,
>>> Jeff (Jiufu)
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.target/powerpc/float128-cmp2-runnable.c: Update requires.
>>>
>>> ---
>>>  gcc/testsuite/gcc.target/powerpc/float128-cmp2-runnable.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/testsuite/gcc.target/powerpc/float128-cmp2-runnable.c b/gcc/testsuite/gcc.target/powerpc/float128-cmp2-runnable.c
>>> index d376a3ca68e..91287c0fb7a 100644
>>> --- a/gcc/testsuite/gcc.target/powerpc/float128-cmp2-runnable.c
>>> +++ b/gcc/testsuite/gcc.target/powerpc/float128-cmp2-runnable.c
>>> @@ -1,5 +1,5 @@
>>>  /* { dg-do run } */
>>> -/* { dg-require-effective-target ppc_float128_sw } */
>>> +/* { dg-require-effective-target ppc_float128_hw } */
>>>  /* { dg-require-effective-target p9vector_hw } */
>>>  /* { dg-options "-O2 -mdejagnu-cpu=power9 " } */
>>>

  reply	other threads:[~2023-04-11  9:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-10  2:09 Jiufu Guo
2023-04-10  9:26 ` Kewen.Lin
2023-04-11  9:14   ` guojiufu
2023-04-11  9:40     ` Kewen.Lin [this message]
2023-04-11 15:13       ` Segher Boessenkool
2023-04-12  5:06         ` Jiufu Guo
2023-04-12  5:31           ` Jiufu Guo
2023-04-12 14:46             ` Michael Meissner
2023-04-13  5:35               ` guojiufu
2023-04-12 12:47         ` Kewen.Lin
2023-04-13  7:42           ` Kewen.Lin
2023-04-13  7:45           ` guojiufu
2023-04-13  8:09             ` Kewen.Lin

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=13ec00da-587b-847d-c26b-98cf463f21ac@linux.ibm.com \
    --to=linkw@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=guojiufu@linux.ibm.com \
    --cc=linkw@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).