From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 6657D3882177; Fri, 14 Jun 2024 02:17:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6657D3882177 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6657D3882177 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.156.1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718331431; cv=none; b=reqMiGlPKKWPHj0QTvA47Tu1+ldrBWJipGPpth8M6+2Q78fjfuuBzroD3HC0NAkhxlkVzd+zZtj1fR+pDiF6S5OaNDoFC9oGI32Uyxan0rv+BJRpL4ktAqP8nHkICedJ3nGlDETfB5asI5SqyR8al7RDAees4KH/38Zvw7DQrYo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718331431; c=relaxed/simple; bh=Lo2r+7iWoOPAQWBFPX1CDOtwKP9+kl9hKpYOIdOG7pA=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=lRCT+ihL8hVwWRJsIhH6ZY2/Yqbv+BQXtDl64LWk+olxGWqKbSHmY+gNg0ACFz2Qn+qm55iAMwxsUiIA2oUsRO7oiyIPVI13Zqah4DWJvb4E6r+sd7cM4AjaUDhWiKKWoFH+6kx18pT2bJDBrsFleUHfcoc10YdakVoH3UsLOAo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353728.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 45DNoHlq017964; Fri, 14 Jun 2024 02:17:05 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h= message-id:date:subject:to:references:cc:from:in-reply-to :content-type:content-transfer-encoding:mime-version; s=pp1; bh= DlvRh3sj0R37CGS9kgGhRlA3cLopMFf9PlDW75gDlDA=; b=QacV182qYMxzvUAI uNdkWewRIbbYJCxfsuC5GzoKuFVxhfM8+MxZzZu4mdPfzzRMiAMgKxAoHhGt9Y2n EDn9vcY4afWiG3ZTR0V27Yv5OLHz8xBVjybUhePSasXOXBs91lFR2+TZjfgWvS/E aRGvS7x6tLp+gWpTk17+eWu2xMUZ+lgqq/SmF4ai5u+Ek9ppoEj02PNapqMMb2oU OVn3Z4z7gBXb08thSf+x3OwmGB2uEtsW6HEwX3iZC1PsgVpJIGJA/LFF65J/b1Jg I79XTjQ2ETGAhUEP+mriupAotv93C9aRvPvpEKLHFgFfx7SdCR79qpsQAQsYAnBT A1cQsw== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3yra1f8bed-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 14 Jun 2024 02:17:04 +0000 (GMT) Received: from m0353728.ppops.net (m0353728.ppops.net [127.0.0.1]) by pps.reinject (8.18.0.8/8.18.0.8) with ESMTP id 45E2H4ZG005831; Fri, 14 Jun 2024 02:17:04 GMT Received: from ppma22.wdc07v.mail.ibm.com (5c.69.3da9.ip4.static.sl-reverse.com [169.61.105.92]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3yra1f8be9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 14 Jun 2024 02:17:04 +0000 (GMT) Received: from pps.filterd (ppma22.wdc07v.mail.ibm.com [127.0.0.1]) by ppma22.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 45E1NLtS027234; Fri, 14 Jun 2024 02:17:02 GMT Received: from smtprelay05.fra02v.mail.ibm.com ([9.218.2.225]) by ppma22.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3yn211dgqt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 14 Jun 2024 02:17:02 +0000 Received: from smtpav04.fra02v.mail.ibm.com (smtpav04.fra02v.mail.ibm.com [10.20.54.103]) by smtprelay05.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 45E2GwbZ51773840 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 14 Jun 2024 02:17:00 GMT Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CBF2C20043; Fri, 14 Jun 2024 02:16:58 +0000 (GMT) Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id F216420040; Fri, 14 Jun 2024 02:16:56 +0000 (GMT) Received: from [9.200.55.24] (unknown [9.200.55.24]) by smtpav04.fra02v.mail.ibm.com (Postfix) with ESMTP; Fri, 14 Jun 2024 02:16:56 +0000 (GMT) Message-ID: Date: Fri, 14 Jun 2024 10:16:55 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH 10/52] jit: Replace uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE To: David Malcolm References: <7c9b2ed53cee1c8c7d5b47abbf963acc2bf5a62e.1717134752.git.linkw@linux.ibm.com> <24355f0ad35a6ce7619be47c0f6cd69624d250cf.camel@redhat.com> Content-Language: en-US Cc: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org, Richard Biener , Joseph Myers From: "Kewen.Lin" In-Reply-To: <24355f0ad35a6ce7619be47c0f6cd69624d250cf.camel@redhat.com> Content-Type: text/plain; charset=UTF-8 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: POeqGHPYM4wBwlIsnQ18plJZqXk5Thzn X-Proofpoint-ORIG-GUID: bFEbSx8ya--Ogsntz2PaznAqJ2uorH9I Content-Transfer-Encoding: 8bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16 definitions=2024-06-13_15,2024-06-13_02,2024-05-17_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 spamscore=0 priorityscore=1501 phishscore=0 clxscore=1011 impostorscore=0 adultscore=0 mlxlogscore=999 suspectscore=0 malwarescore=0 mlxscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2405170001 definitions=main-2406140010 X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi David, on 2024/6/13 21:44, David Malcolm wrote: > On Sun, 2024-06-02 at 22:01 -0500, Kewen Lin wrote: >> Joseph pointed out "floating types should have their mode, >> not a poorly defined precision value" in the discussion[1], >> as he and Richi suggested, the existing macros >> {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a >> hook mode_for_floating_type.  Unlike the other FEs, for the >> uses in recording::memento_of_get_type::get_size, since >> {float,{,long_}double}_type_node haven't been initialized >> yet, this is to replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE >> with calling hook targetm.c.mode_for_floating_type. >> >> [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html >> >> gcc/jit/ChangeLog: >> >>         * jit-recording.cc >> (recording::memento_of_get_type::get_size): Update >>         macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE by calling >>         targetm.c.mode_for_floating_type with >>         TI_{FLOAT,DOUBLE,LONG_DOUBLE}_TYPE. >> --- >>  gcc/jit/jit-recording.cc | 12 ++++++++---- >>  1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc >> index 68a2e860c1f..7719b898e57 100644 >> --- a/gcc/jit/jit-recording.cc >> +++ b/gcc/jit/jit-recording.cc >> @@ -21,7 +21,7 @@ along with GCC; see the file COPYING3.  If not see >>  #include "config.h" >>  #include "system.h" >>  #include "coretypes.h" >> -#include "tm.h" >> +#include "target.h" >>  #include "pretty-print.h" >>  #include "toplev.h" >>   >> @@ -2353,6 +2353,7 @@ size_t >>  recording::memento_of_get_type::get_size () >>  { >>    int size; >> +  machine_mode m; >>    switch (m_kind) >>      { >>      case GCC_JIT_TYPE_VOID: >> @@ -2399,13 +2400,16 @@ recording::memento_of_get_type::get_size () >>        size = 128; >>        break; >>      case GCC_JIT_TYPE_FLOAT: >> -      size = FLOAT_TYPE_SIZE; >> +      m = targetm.c.mode_for_floating_type (TI_FLOAT_TYPE); >> +      size = GET_MODE_PRECISION (m).to_constant (); >>        break; >>      case GCC_JIT_TYPE_DOUBLE: >> -      size = DOUBLE_TYPE_SIZE; >> +      m = targetm.c.mode_for_floating_type (TI_DOUBLE_TYPE); >> +      size = GET_MODE_PRECISION (m).to_constant (); >>        break; >>      case GCC_JIT_TYPE_LONG_DOUBLE: >> -      size = LONG_DOUBLE_TYPE_SIZE; >> +      m = targetm.c.mode_for_floating_type (TI_LONG_DOUBLE_TYPE); >> +      size = GET_MODE_PRECISION (m).to_constant (); >>        break; >>      case GCC_JIT_TYPE_SIZE_T: >>        size = MAX_BITS_PER_WORD; > > [CCing jit mailing list] > > Thanks for the patch; sorry for the delay in responding. > > Did your testing include jit? Note that --enable-languages=all does > *not* include it (due to it needing --enable-host-shared). Thanks for the hints! Yes, as noted in the cover letter, I did test jit. Initially I used TYPE_PRECISION ({float,{long_,}double_type_node) to replace these just like what I proposed for the other FE changes, but the testing showed some failures on test-combination.c etc., by looking into them, I realized that this call recording::memento_of_get_type::get_size can happen before when we set up those type nodes. Then I had to use the current approach with the new hook, it made all failures gone (no regressions). btw, test result comparison showed some more lines with "NA->PASS: test-threads.c.exe", since it's positive, I didn't look into it. > > The jit::recording code runs *very* early - before toplev::main. For > example, a call to gcc_jit_type_get_size can trigger the above code > path before toplev::main has run. > > target.h says each target should have a: > > struct gcc_target targetm = TARGET_INITIALIZER; > > Has targetm.c.mode_for_floating_type been initialized enough by that > static initialization? It depends on how to define "enough". The hook has been initialized as you pointed out, I just debugged it and confirmed target specific hook was called as expected (rs6000_c_mode_for_floating_type on Power) when this jit::recording function gets called. If "enough" refers to something like command line options, it's not ready. > Could the mode_for_floating_type hook be > relying on some target-specific dynamic initialization that hasn't run > yet? (e.g. taking account of command-line options?) > Yes, it could. Like rs6000 port, the hook checks rs6000_long_double_type_size for long double (it's related to command line option -mlong-double-x) and some other targets like i386, also would like to check TARGET_LONG_DOUBLE_64 and TARGET_LONG_DOUBLE_128. But I think it isn't worse than before, without this change (with the previous macro), we used to define the macro with the things related to this command line options, which are still not ready. #define LONG_DOUBLE_TYPE_SIZE rs6000_long_double_type_size I debugged the code, jit::recording will see rs6000_long_double_type_size with the static initialized value zero, it means that the function recording::memento_of_get_type::get_size would get zero byte size for the type (I assume that it's unexpected for the code?). With this new hook, although it can provide not exact type size (can be off from the one specified by command line), it returns a reasonable size (comparing with the zero size). From this perspective, it's slightly better? + if (ti == TI_LONG_DOUBLE_TYPE) + return rs6000_long_double_type_size == FLOAT_PRECISION_TFmode ? TFmode + : DFmode; BR, Kewen