From patchwork Mon Nov 19 10:27:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 999699 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-490405-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="L2viqCb0"; 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 42z4ld0Hnlz9s7T for ; Mon, 19 Nov 2018 21:27:23 +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 :content-transfer-encoding; q=dns; s=default; b=UrlgNGJW9UPJzsP6 jgKLJ6rKb1lC4/zLW7WjoUI/s6vG+r5W9UC8BKTSEedmFFbBKugDGD5Ai8eXaBG/ d5wNGCUkiMJ90lBgP8Uu87Q2KOZcDbvPbr8XR8752UxuGWImTXdFxZCGUVFa6Pnx jt4DhVLShrETClGiOz6wRLlMhlg= 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 :content-transfer-encoding; s=default; bh=TLKxkQkck9YfgQHXgcQbrb 3G8Bc=; b=L2viqCb0uWaifWaYPUyW+XIWDMyboN3NOCaO1its9kpczRW7zA59Q8 54H6mQ/OUOsHDvGgp2d3ggjt55iXFQ50yGo//rXGfz6V8gTEhAyH8nfk2hk+o77Y 5u3STztOozvC0NZ0Bk9vLRHY+ElcbB+ubOd47T+Xv3px1CLMzZjCs= Received: (qmail 79075 invoked by alias); 19 Nov 2018 10:27:15 -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 79047 invoked by uid 89); 19 Nov 2018 10:27:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=UD:l, ccc, CCC, const0_rtx X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Nov 2018 10:27:13 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id E14FC8139A for ; Mon, 19 Nov 2018 11:27:10 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 77kv7AwKOK_L for ; Mon, 19 Nov 2018 11:27:10 +0100 (CET) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id AFE5681397 for ; Mon, 19 Nov 2018 11:27:10 +0100 (CET) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: Fix bad interaction between ce and cmpelim passes on Visium Date: Mon, 19 Nov 2018 11:27:08 +0100 Message-ID: <1780456.IWWpvtJA7H@polaris> MIME-Version: 1.0 This is a code quality regression that appeared during GCC 8 development on Visium in the form of the failure of gcc.target/visium/overflow*.c tests and that I swept under the rug back then. On architectures which do not provide scc instructions like Visium, you can nevertheless implement a few variants of cstore by using the carry, which is traditionally modeled as sltu, and some tricks (for example, on Visium, we implement seq/sne/sltu/sgtu/sleu/sgeu this way); the only counterpart is that the patterns are a bit convoluted if you want to have exact RTL semantics. So generating cstore instructions, usually done by the if-conversion pass, can disable the elimination of redundant comparison instructions, only done by the compare-elim pass on some RISC architectures, because the latter pass is based on generic pattern matching. That's what happened here for UNEGV operations. The regression is fixed by teaching the compare-elim pass to handle a NOT in the first operand of comparisons and adjusting the neg2_insn_set_carry of the Visium to make it consistent with the unegv3 pattern. Tested on visium-elf, sparc-sun-solaris2.11 and x86_64-suse-linux, applied on the mainline. 2018-11-19 Eric Botcazou * compare-elim.c (struct comparison): Add not_in_a field. (is_not): New static function. (strip_not): Likewise. (conforming_compare): Handle a NOT in the first operand. (can_eliminate_compare): Likewise. (find_comparison_dom_walker::before_dom_children): Likewise. (try_eliminate_compare): Likewise. * config/visium/visium.md (negsi2_insn_set_carry): Turn into... (neg2_insn_set_carry): ...this and add missing NEG operation. 2018-11-19 Eric Botcazou * gcc.target/visium/overflow8.c: Remove -fno-if-conversion and unrelated final test. * gcc.target/visium/overflow16: Likewise. * gcc.target/visium/overflow32.c: Likewise. Index: compare-elim.c =================================================================== --- compare-elim.c (revision 266178) +++ compare-elim.c (working copy) @@ -123,10 +123,32 @@ struct comparison /* True if its inputs are still valid at the end of the block. */ bool inputs_valid; + + /* Whether IN_A is wrapped in a NOT before being compared. */ + bool not_in_a; }; static vec all_compares; +/* Return whether X is a NOT unary expression. */ + +static bool +is_not (rtx x) +{ + return GET_CODE (x) == NOT; +} + +/* Strip a NOT unary expression around X, if any. */ + +static rtx +strip_not (rtx x) +{ + if (is_not (x)) + return XEXP (x, 0); + + return x; +} + /* Look for a "conforming" comparison, as defined above. If valid, return the rtx for the COMPARE itself. */ @@ -147,7 +169,7 @@ conforming_compare (rtx_insn *insn) if (!REG_P (dest) || REGNO (dest) != targetm.flags_regnum) return NULL; - if (!REG_P (XEXP (src, 0))) + if (!REG_P (strip_not (XEXP (src, 0)))) return NULL; if (CONSTANT_P (XEXP (src, 1)) || REG_P (XEXP (src, 1))) @@ -278,10 +300,13 @@ can_eliminate_compare (rtx compare, rtx return false; /* Make sure the compare is redundant with the previous. */ - if (!rtx_equal_p (XEXP (compare, 0), cmp->in_a) + if (!rtx_equal_p (strip_not (XEXP (compare, 0)), cmp->in_a) || !rtx_equal_p (XEXP (compare, 1), cmp->in_b)) return false; + if (is_not (XEXP (compare, 0)) != cmp->not_in_a) + return false; + /* New mode must be compatible with the previous compare mode. */ machine_mode new_mode = targetm.cc_modes_compatible (GET_MODE (compare), cmp->orig_mode); @@ -365,8 +390,9 @@ find_comparison_dom_walker::before_dom_c last_cmp = XCNEW (struct comparison); last_cmp->insn = insn; last_cmp->prev_clobber = last_clobber; - last_cmp->in_a = XEXP (src, 0); + last_cmp->in_a = strip_not (XEXP (src, 0)); last_cmp->in_b = XEXP (src, 1); + last_cmp->not_in_a = is_not (XEXP (src, 0)); last_cmp->eh_note = eh_note; last_cmp->orig_mode = GET_MODE (src); if (last_cmp->in_b == const0_rtx @@ -837,7 +863,9 @@ try_eliminate_compare (struct comparison flags = gen_rtx_REG (cmp->orig_mode, targetm.flags_regnum); /* Generate a new comparison for installation in the setter. */ - rtx y = copy_rtx (cmp_a); + rtx y = cmp->not_in_a + ? gen_rtx_NOT (GET_MODE (cmp_a), copy_rtx (cmp_a)) + : copy_rtx (cmp_a); y = gen_rtx_COMPARE (GET_MODE (flags), y, copy_rtx (cmp_b)); y = gen_rtx_SET (flags, y); Index: config/visium/visium.md =================================================================== --- config/visium/visium.md (revision 266178) +++ config/visium/visium.md (working copy) @@ -1208,14 +1208,14 @@ (define_insn "*neg2_insn %0,r0,%1" [(set_attr "type" "arith")]) -(define_insn "negsi2_insn_set_carry" +(define_insn "neg2_insn_set_carry" [(set (reg:CCC R_FLAGS) - (compare:CCC (not:SI (match_operand:SI 1 "register_operand" "r")) + (compare:CCC (not:I (neg:I (match_operand:I 1 "register_operand" "r"))) (const_int -1))) - (set (match_operand:SI 0 "register_operand" "=r") - (neg:SI (match_dup 1)))] + (set (match_operand:I 0 "register_operand" "=r") + (neg:I (match_dup 1)))] "reload_completed" - "sub.l %0,r0,%1" + "sub %0,r0,%1" [(set_attr "type" "arith")]) (define_insn "*neg2_insn_set_overflow" Index: testsuite/gcc.target/visium/overflow16.c =================================================================== --- testsuite/gcc.target/visium/overflow16.c (revision 266178) +++ testsuite/gcc.target/visium/overflow16.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fno-if-conversion" } */ +/* { dg-options "-O2" } */ #include @@ -36,4 +36,3 @@ bool my_neg_overflow (short a, short *re /* { dg-final { scan-assembler-times "add.w" 2 } } */ /* { dg-final { scan-assembler-times "sub.w" 4 } } */ /* { dg-final { scan-assembler-not "cmp.w" } } */ -/* { dg-final { scan-assembler-not "mov.w" } } */ Index: testsuite/gcc.target/visium/overflow32.c =================================================================== --- testsuite/gcc.target/visium/overflow32.c (revision 266178) +++ testsuite/gcc.target/visium/overflow32.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fno-if-conversion" } */ +/* { dg-options "-O2" } */ #include @@ -36,4 +36,3 @@ bool my_neg_overflow (int a, int *res) /* { dg-final { scan-assembler-times "add.l" 2 } } */ /* { dg-final { scan-assembler-times "sub.l" 4 } } */ /* { dg-final { scan-assembler-not "cmp.l" } } */ -/* { dg-final { scan-assembler-not "mov.l" } } */ Index: testsuite/gcc.target/visium/overflow8.c =================================================================== --- testsuite/gcc.target/visium/overflow8.c (revision 266178) +++ testsuite/gcc.target/visium/overflow8.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fno-if-conversion" } */ +/* { dg-options "-O2" } */ #include @@ -36,4 +36,3 @@ bool my_neg_overflow (signed char a, sig /* { dg-final { scan-assembler-times "add.b" 2 } } */ /* { dg-final { scan-assembler-times "sub.b" 4 } } */ /* { dg-final { scan-assembler-not "cmp.b" } } */ -/* { dg-final { scan-assembler-not "mov.b" } } */