From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id F39693857C69 for ; Wed, 2 Dec 2020 02:10:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F39693857C69 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 0B222WDS175819; Tue, 1 Dec 2020 21:10:09 -0500 Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 355j4fxnng-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 01 Dec 2020 21:10:07 -0500 Received: from m0098416.ppops.net (m0098416.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 0B222vHb177250; Tue, 1 Dec 2020 21:10:07 -0500 Received: from ppma04ams.nl.ibm.com (63.31.33a9.ip4.static.sl-reverse.com [169.51.49.99]) by mx0b-001b2d01.pphosted.com with ESMTP id 355j4fxnmq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 01 Dec 2020 21:10:06 -0500 Received: from pps.filterd (ppma04ams.nl.ibm.com [127.0.0.1]) by ppma04ams.nl.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 0B227ocM012671; Wed, 2 Dec 2020 02:10:05 GMT Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by ppma04ams.nl.ibm.com with ESMTP id 353e683qnt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 02 Dec 2020 02:10:05 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 0B22A32r64094702 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 2 Dec 2020 02:10:03 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DC16511C050; Wed, 2 Dec 2020 02:10:02 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8204B11C04A; Wed, 2 Dec 2020 02:10:02 +0000 (GMT) Received: from localhost.localdomain (unknown [9.145.42.202]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 2 Dec 2020 02:10:02 +0000 (GMT) From: Ilya Leoshkevich To: Jeff Law , gcc-patches@gcc.gnu.org Cc: Marc Glisse , Richard Biener , Jakub Jelinek , Ilya Leoshkevich Subject: [PATCH RESEND] tree-ssa-threadbackward.c (profitable_jump_thread_path): Do not allow __builtin_constant_p. Date: Wed, 2 Dec 2020 03:09:59 +0100 Message-Id: <20201202020959.574839-1-iii@linux.ibm.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: <8371e7e5-5e6d-561d-0a95-c67de6af4e96@redhat.com> References: <8371e7e5-5e6d-561d-0a95-c67de6af4e96@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312, 18.0.737 definitions=2020-12-01_12:2020-11-30, 2020-12-01 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 bulkscore=0 adultscore=0 lowpriorityscore=0 mlxlogscore=999 malwarescore=0 priorityscore=1501 clxscore=1015 spamscore=0 impostorscore=0 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2012020006 X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, 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: Wed, 02 Dec 2020 02:10:11 -0000 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. --- .../s390/builtin-constant-p-threading.c | 46 +++++++++++++++++++ gcc/tree-ssa-threadbackward.c | 7 ++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/s390/builtin-constant-p-threading.c diff --git a/gcc/testsuite/gcc.target/s390/builtin-constant-p-threading.c b/gcc/testsuite/gcc.target/s390/builtin-constant-p-threading.c new file mode 100644 index 00000000000..5f0acdce0b0 --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/builtin-constant-p-threading.c @@ -0,0 +1,46 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=z196 -mzarch" } */ + +typedef struct +{ + int counter; +} atomic_t; + +static inline __attribute__ ((__gnu_inline__)) int +__atomic_add (int val, int *ptr) +{ + int old; + asm volatile("laa %[old],%[val],%[ptr]\n" + : [old] "=d" (old), [ptr] "+Q"(*ptr) + : [val] "d" (val) + : "cc", "memory"); + return old; +} + +static inline __attribute__ ((__gnu_inline__)) void +__atomic_add_const (int val, int *ptr) +{ + asm volatile("asi %[ptr],%[val]\n" + : [ptr] "+Q" (*ptr) + : [val] "i" (val) + : "cc", "memory"); +} + +static inline __attribute__ ((__gnu_inline__)) void +atomic_add (int i, atomic_t *v) +{ + if (__builtin_constant_p (i) && (i > -129) && (i < 128)) + { + __atomic_add_const (i, &v->counter); + return; + } + __atomic_add (i, &v->counter); +} + +static atomic_t num_active_cpus = { (0) }; + +void +ledtrig_cpu (_Bool is_active) +{ + atomic_add (is_active ? 1 : -1, &num_active_cpus); +} diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c index 327628f1662..30f692672d9 100644 --- a/gcc/tree-ssa-threadbackward.c +++ b/gcc/tree-ssa-threadbackward.c @@ -259,8 +259,13 @@ thread_jumps::profitable_jump_thread_path (basic_block bbi, tree name, !gsi_end_p (gsi); gsi_next_nondebug (&gsi)) { + /* Do not allow OpenACC loop markers and __builtin_constant_p on + threading paths. The latter is disallowed, because an + expression might be constant on two threading paths, and + become non-constant (i.e.: phi) when they merge. */ gimple *stmt = gsi_stmt (gsi); - if (gimple_call_internal_p (stmt, IFN_UNIQUE)) + if (gimple_call_internal_p (stmt, IFN_UNIQUE) + || gimple_call_builtin_p (stmt, BUILT_IN_CONSTANT_P)) { m_path.pop (); return NULL; -- 2.25.4