From patchwork Mon Jan 20 12:54:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 1225931 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-517765-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=bFhihxlL; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 481WpL4BNXz9sRK for ; Mon, 20 Jan 2020 23:54:34 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type; q=dns; s= default; b=OxaDbE0xT3QKTq+/F1YLY1ZS+L9VkLEdvWCCDYM3/i4N7lwxSLlmR 39344mdWdcjsss6yqnH0Soy2zP6xTRERLGD2p0LhxfaP9LlwQZu9qb6RoMuIegP6 sciFTcGVmFnb9MlqmvlbZ5blfvggrbvuTSIn24A2nsED5UxTznE3UQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type; s= default; bh=p8alncANw58C2ZwU6tE4sxoiwJg=; b=bFhihxlLNO3pH2YmQl9p 1fe6sHmYvsnXWCTWg+M3kENgPE+axvb+dY92czFsa5HOmJJpKp+JEIN9Mlm1SPmj +vkYJS/PfdX1zhyvc9tSN8h+B6Jv6gnwO3REkb6LcWStQr8foEG+UTNDWYeAqYi/ KUkXEsxrKy5OYTM708AU4yA= Received: (qmail 84931 invoked by alias); 20 Jan 2020 12:54:26 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 84909 invoked by uid 89); 20 Jan 2020 12:54:25 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=intentionally X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Jan 2020 12:54:15 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F156830E for ; Mon, 20 Jan 2020 04:54:13 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9A8BE3F52E for ; Mon, 20 Jan 2020 04:54:13 -0800 (PST) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: [PATCH] cselib: Fix handling of multireg values for call insns [PR93170] Date: Mon, 20 Jan 2020 12:54:12 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes g:3bd2918594dae34ae84f mishandled the case in which only the tail end of a multireg hard register is invalidated by the call. Walking all the entries should be both safer and more precise. Avoiding cselib_invalidate_regno also means that we no longer walk the same list multiple times (which is something we did before g:3bd2918594dae34ae84f too). Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Richard 2020-01-20 Richard Sandiford gcc/ PR rtl-optimization/93170 * cselib.c (cselib_invalidate_regno_val): New function, split out from... (cselib_invalidate_regno): ...here. (cselib_invalidated_by_call_p): New function. (cselib_process_insn): Iterate over all the hard-register entries in REG_VALUES and invalidate any that cross call-clobbered registers. gcc/testsuite/ * gcc.dg/torture/pr93170.c: New test. --- gcc/cselib.c | 139 ++++++++++++++----------- gcc/testsuite/gcc.dg/torture/pr93170.c | 33 ++++++ 2 files changed, 113 insertions(+), 59 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr93170.c diff --git a/gcc/cselib.c b/gcc/cselib.c index 845f79b99ee..3e0c69d67b8 100644 --- a/gcc/cselib.c +++ b/gcc/cselib.c @@ -2156,6 +2156,52 @@ cselib_lookup (rtx x, machine_mode mode, return ret; } +/* Invalidate the value at *L, which is part of REG_VALUES (REGNO). */ + +static void +cselib_invalidate_regno_val (unsigned int regno, struct elt_list **l) +{ + cselib_val *v = (*l)->elt; + if (*l == REG_VALUES (regno)) + { + /* Maintain the invariant that the first entry of + REG_VALUES, if present, must be the value used to set + the register, or NULL. This is also nice because + then we won't push the same regno onto user_regs + multiple times. */ + (*l)->elt = NULL; + l = &(*l)->next; + } + else + unchain_one_elt_list (l); + + v = canonical_cselib_val (v); + + bool had_locs = v->locs != NULL; + rtx_insn *setting_insn = v->locs ? v->locs->setting_insn : NULL; + + /* Now, we clear the mapping from value to reg. It must exist, so + this code will crash intentionally if it doesn't. */ + for (elt_loc_list **p = &v->locs; ; p = &(*p)->next) + { + rtx x = (*p)->loc; + + if (REG_P (x) && REGNO (x) == regno) + { + unchain_one_elt_loc_list (p); + break; + } + } + + if (had_locs && v->locs == 0 && !PRESERVED_VALUE_P (v->val_rtx)) + { + if (setting_insn && DEBUG_INSN_P (setting_insn)) + n_useless_debug_values++; + else + n_useless_values++; + } +} + /* Invalidate any entries in reg_values that overlap REGNO. This is called if REGNO is changing. MODE is the mode of the assignment to REGNO, which is used to determine how many hard registers are being changed. If MODE @@ -2202,9 +2248,6 @@ cselib_invalidate_regno (unsigned int regno, machine_mode mode) while (*l) { cselib_val *v = (*l)->elt; - bool had_locs; - rtx_insn *setting_insn; - struct elt_loc_list **p; unsigned int this_last = i; if (i < FIRST_PSEUDO_REGISTER && v != NULL) @@ -2219,44 +2262,7 @@ cselib_invalidate_regno (unsigned int regno, machine_mode mode) } /* We have an overlap. */ - if (*l == REG_VALUES (i)) - { - /* Maintain the invariant that the first entry of - REG_VALUES, if present, must be the value used to set - the register, or NULL. This is also nice because - then we won't push the same regno onto user_regs - multiple times. */ - (*l)->elt = NULL; - l = &(*l)->next; - } - else - unchain_one_elt_list (l); - - v = canonical_cselib_val (v); - - had_locs = v->locs != NULL; - setting_insn = v->locs ? v->locs->setting_insn : NULL; - - /* Now, we clear the mapping from value to reg. It must exist, so - this code will crash intentionally if it doesn't. */ - for (p = &v->locs; ; p = &(*p)->next) - { - rtx x = (*p)->loc; - - if (REG_P (x) && REGNO (x) == i) - { - unchain_one_elt_loc_list (p); - break; - } - } - - if (had_locs && v->locs == 0 && !PRESERVED_VALUE_P (v->val_rtx)) - { - if (setting_insn && DEBUG_INSN_P (setting_insn)) - n_useless_debug_values++; - else - n_useless_values++; - } + cselib_invalidate_regno_val (i, l); } } } @@ -2714,6 +2720,28 @@ fp_setter_insn (rtx_insn *insn) return true; } +/* V is one of the values in REG_VALUES (REGNO). Return true if it + would be invalidated by CALLEE_ABI. */ + +static bool +cselib_invalidated_by_call_p (const function_abi &callee_abi, + unsigned int regno, cselib_val *v) +{ + machine_mode mode = GET_MODE (v->val_rtx); + if (mode == VOIDmode) + { + v = REG_VALUES (regno)->elt; + if (!v) + /* If we don't know what the mode of the constant value is, and we + don't know what mode the register was set in, conservatively + assume that the register is clobbered. The value's going to be + essentially useless in this case anyway. */ + return true; + mode = GET_MODE (v->val_rtx); + } + return callee_abi.clobbers_reg_p (mode, regno); +} + /* Record the effects of INSN. */ void @@ -2748,24 +2776,17 @@ cselib_process_insn (rtx_insn *insn) { function_abi callee_abi = insn_callee_abi (insn); for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) - if (elt_list *values = REG_VALUES (i)) - { - /* If we know what mode the value was set in, check whether - it is still available after the call in that mode. If we - don't know the mode, we have to check for the worst-case - scenario instead. */ - if (values->elt) - { - machine_mode mode = GET_MODE (values->elt->val_rtx); - if (callee_abi.clobbers_reg_p (mode, i)) - cselib_invalidate_regno (i, mode); - } - else - { - if (callee_abi.clobbers_at_least_part_of_reg_p (i)) - cselib_invalidate_regno (i, reg_raw_mode[i]); - } - } + { + elt_list **l = ®_VALUES (i); + while (*l) + { + cselib_val *v = (*l)->elt; + if (v && cselib_invalidated_by_call_p (callee_abi, i, v)) + cselib_invalidate_regno_val (i, l); + else + l = &(*l)->next; + } + } /* Since it is not clear how cselib is going to be used, be conservative here and treat looping pure or const functions diff --git a/gcc/testsuite/gcc.dg/torture/pr93170.c b/gcc/testsuite/gcc.dg/torture/pr93170.c new file mode 100644 index 00000000000..25a93a3743e --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr93170.c @@ -0,0 +1,33 @@ +/* { dg-do run } */ +/* { dg-require-effective-target int128 } */ +/* { dg-additional-options "-frename-registers -fno-tree-forwprop -fno-tree-fre -fira-algorithm=priority -mstringop-strategy=loop --param=hot-bb-frequency-fraction=0 -Wno-psabi" { target { x86_64-*-* i?86-*-* } } } */ + +typedef unsigned char v64u8 __attribute__ ((vector_size (64))); +typedef unsigned short v64u16 __attribute__ ((vector_size (64))); +typedef unsigned int v64u32 __attribute__ ((vector_size (64))); +typedef unsigned long long v64u64 __attribute__ ((vector_size (64))); +typedef unsigned __int128 u128; +typedef unsigned __int128 v64u128 __attribute__ ((vector_size (64))); + +int a, b, d, e; +v64u64 c; + +v64u128 +foo (u128 g, v64u16 h, v64u32 i, v64u128 j) +{ + c[e] = 0; + j &= (i[1] <<= b); + j >>= ((v64u128) h <= j); + d = __builtin_popcountll (-((v64u8) i)[0]); + return a + g + j; +} + +int +main (void) +{ + v64u128 x = foo (0, (v64u16) { 0, 0, 0, 0, 0, 0, 0, 0, 5 }, (v64u32) { 2 }, + (v64u128) { }); + if (x[0] || x[1] || x[2] || x[3]) + __builtin_abort (); + return 0; +}