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 E655B396EC67 for ; Thu, 3 Dec 2020 09:52:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E655B396EC67 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 0B39Wn6X111619; Thu, 3 Dec 2020 04:52:55 -0500 Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 356w89s63m-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 03 Dec 2020 04:52:54 -0500 Received: from m0098394.ppops.net (m0098394.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 0B39XQIn115540; Thu, 3 Dec 2020 04:52:54 -0500 Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0a-001b2d01.pphosted.com with ESMTP id 356w89s621-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 03 Dec 2020 04:52:54 -0500 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 0B39X3g1004913; Thu, 3 Dec 2020 09:52:52 GMT Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by ppma03ams.nl.ibm.com with ESMTP id 353e68d38q-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 03 Dec 2020 09:52:52 +0000 Received: from d06av24.portsmouth.uk.ibm.com (d06av24.portsmouth.uk.ibm.com [9.149.105.60]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 0B39qoqL6882020 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 3 Dec 2020 09:52:50 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E7F6C42042; Thu, 3 Dec 2020 09:52:49 +0000 (GMT) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 946F14203F; Thu, 3 Dec 2020 09:52:49 +0000 (GMT) Received: from sig-9-145-76-141.uk.ibm.com (unknown [9.145.76.141]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 3 Dec 2020 09:52:49 +0000 (GMT) Message-ID: Subject: Re: [PATCH RESEND] tree-ssa-threadbackward.c (profitable_jump_thread_path): Do not allow __builtin_constant_p. From: Ilya Leoshkevich To: Jeff Law , gcc-patches@gcc.gnu.org Cc: Marc Glisse , Richard Biener , Jakub Jelinek Date: Thu, 03 Dec 2020 10:52:49 +0100 In-Reply-To: <64f85aa5-26b6-0618-ccda-824091e4749b@redhat.com> References: <8371e7e5-5e6d-561d-0a95-c67de6af4e96@redhat.com> <20201202020959.574839-1-iii@linux.ibm.com> <64f85aa5-26b6-0618-ccda-824091e4749b@redhat.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.34.4 (3.34.4-1.fc31) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312, 18.0.737 definitions=2020-12-03_06:2020-12-03, 2020-12-03 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1011 phishscore=0 suspectscore=0 bulkscore=0 spamscore=0 malwarescore=0 lowpriorityscore=0 priorityscore=1501 adultscore=0 mlxscore=0 mlxlogscore=999 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2012030058 X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Dec 2020 09:52:57 -0000 On Wed, 2020-12-02 at 11:42 -0700, Jeff Law wrote: > > On 12/1/20 7:09 PM, Ilya Leoshkevich wrote: > > On Tue, 2020-12-01 at 15:34 -0700, Jeff Law wrote: > > > No strong opinions. I think whichever is less invasive in terms > > > of > > > code > > > quality is probably the way to go. What we want to avoid is > > > suppressing > > > threading unnecessarily as that often leads to false positives > > > from > > > middle-end based warnings. Suppressing threading can also lead > > > to > > > build > > > failures in the kernel due to the way they use b_c_p. > > I think v1 is better then. Would you mind approving the following? > > That's the same code as in v1, but with the improved commit message > > and > > comments. > > > > > > > > Linux Kernel (specifically, drivers/leds/trigger/ledtrig-cpu.c) > > build > > with GCC 10 fails on s390 with "impossible constraint". > > > > Explanation by Jeff Law: > > > > ``` > > So what we have is a b_c_p at the start of an if-else > > chain. Subsequent > > tests on the "true" arm of the the b_c_p test may throw us off the > > constant path (because the constants are out of range). Once all > > the > > tests are passed (it's constant and the constant is in range) the > > true > > arm's terminal block has a special asm that requires a constant > > argument. In the case where we get to the terminal block on the > > true > > arm, the argument to the b_c_p is used as the constant argument to > > the > > special asm. > > > > At first glace jump threading seems to be doing the right > > thing. Except > > that we end up with two paths to that terminal block with the > > special > > asm, one for each of the two constant arguments to the b_c_p call. > > Naturally since that same value is used in the asm, we have to > > introduce > > a PHI to select between them at the head of the terminal > > block. Now > > the argument in the asm is no longer constant and boom we fail. > > ``` > > > > Fix by disallowing __builtin_constant_p on threading paths. > > > > gcc/ChangeLog: > > > > 2020-06-03 Ilya Leoshkevich > > > > * tree-ssa-threadbackward.c > > (thread_jumps::profitable_jump_thread_path): > > Do not allow __builtin_constant_p on a threading path. > > > > gcc/testsuite/ChangeLog: > > > > 2020-06-03 Ilya Leoshkevich > > > > * gcc.target/s390/builtin-constant-p-threading.c: New test. > OK. I think the old forward threader has the same problem. Which I > think can be fixed by returning NULL from > record_temporary_equivalences_from_stmts_at_dest when we see the > B_C_P > call. Fixing that in the obvious way is pre-approved once it's gone > through the usual testing. Thanks! I've committed both: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=70a62009181f66d1d1c90d3c74de38e153c96eb0 https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=614aff0adf8fba5d843ec894603160151c20f0aa Best regards, Ilya