From patchwork Tue Nov 20 13:42:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Matz X-Patchwork-Id: 1000507 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-490515-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="mUaSmX/D"; 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 42zn2b5Wp2z9s1x for ; Wed, 21 Nov 2018 00:42:45 +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:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=jckX7oV7WVyjUqplKaLtruouGFaBBkiaTfLnp6DUcc0k7fHxr93/0 nAIwIXjMmUV2NThalBu8ogy9jVNBcbjRKH0yNMZma5SvfCIUhoRZTL6xxEdtTjPg jZ0ftXoqVcwNZNQg8aCRcBayZvQV5jHdS0Mobh1iQ0vii+JaqPh6ss= 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:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=8rcd2lxX4xoJ1EpOlVxBpsC/v5A=; b=mUaSmX/DNl6TI0A4An00 8KEHqT5AcxBOWn0RHyEwtSoCf34i181Pwqn1fpYYoIeuG8vQFc0PpWhIPU1Mecqg 4VFpO3VeuRn8y8rjOlIxiuKXj74mIkbubKu/TUbjJCrgxc0K4/DaPyEmhZRSyKHd IvS/kttk0HV5HN4ZQgBMpTc= Received: (qmail 99295 invoked by alias); 20 Nov 2018 13:42:38 -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 99213 invoked by uid 89); 20 Nov 2018 13:42:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy=horse, minds, 45837 X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Nov 2018 13:42:34 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 6A1E2AF9B for ; Tue, 20 Nov 2018 13:42:32 +0000 (UTC) Date: Tue, 20 Nov 2018 13:42:32 +0000 (UTC) From: Michael Matz To: gcc-patches@gcc.gnu.org Subject: Fix PR37916 (unnecessary spilling) Message-ID: User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-IsSubscribed: yes Hi, this bug report is about cris generating worse code since tree-ssa. The effect is also visible on x86-64. The symptom is that the work horse of adler32.c (from zlib) needs spills in the inner loop, while gcc 3 did not, and those spills go away with -fno-tree-reassoc. The underlying reason for the spills is register pressure, which could either be rectified by the pressure aware scheduling (which cris doesn't have), or by simply not generating high pressure code to begin with. In this case it's TER which ultimately causes the register pressure to increase, and there are many plans in people minds how to fix this (make TER regpressure aware, do some regpressure scheduling on gimple, or even more ambitious things), but this patch doesn't tackle this. Instead it makes reassoc not generate the situation which causes TER to run wild. TER increasing register pressure is a long standing problem and so it has some heuristics to avoid that. One wobbly heuristic is that it doesn't TER expressions together that have the same base variable as their LHSs. But reassoc generates only anonymous SSA names for its temporary subexpressions, so that TER heuristic doesn't apply. In this testcase it's even the case that reassoc doesn't really change much code (one addition moves from the end to the beginning of the inner loop), so that whole rewriting is even pointless. In any case, let's use copy_ssa_name instead of make_ssa_name, when we have an obvious LHS; that leads to TER making the same decisions with and without -fno-tree-reassoc, leading to the same register pressure and no spills. On x86-64 the effect is: before patch: 48 bytes stackframe, 24 stack accesses (most of them in the loops), 576 bytes codesize; after patch: no stack frame, no stack accesses, 438 bytes codesize On cris: before patch: 64 bytes stack frame, 27 stack access in loops, size of .s 145 lines after patch: 20 bytes stack frame (as it uses callee saved regs, which is also complained about in the bug report), but no stack accesses in loops, size of .s: 125 lines I'm wondering about testcase: should I add an x86-64 specific that tests for no stack accesses, or would that be too constraining in the future? Regstrapped on x86-64-linux, no regressions. Okay for trunk? Ciao, Michael. PR tree-optimization/37916 * tree-ssa-reassoc.c (make_new_ssa_for_def): Use copy_ssa_name. (rewrite_expr_tree, linearize_expr, negate_value, repropagate_negates, attempt_builtin_copysign, reassociate_bb): Likewise. diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c index 971d926e7895..339c3d4e447f 100644 --- a/gcc/tree-ssa-reassoc.c +++ b/gcc/tree-ssa-reassoc.c @@ -1182,7 +1182,7 @@ make_new_ssa_for_def (gimple *stmt, enum tree_code opcode, tree op) tree new_lhs, new_debug_lhs = NULL_TREE; tree lhs = gimple_get_lhs (stmt); - new_lhs = make_ssa_name (TREE_TYPE (lhs)); + new_lhs = copy_ssa_name (lhs); gimple_set_lhs (stmt, new_lhs); /* Also need to update GIMPLE_DEBUGs. */ @@ -4512,7 +4512,7 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex, { gimple *insert_point = find_insert_point (stmt, oe1->op, oe2->op); - lhs = make_ssa_name (TREE_TYPE (lhs)); + lhs = copy_ssa_name (lhs); stmt = gimple_build_assign (lhs, gimple_assign_rhs_code (stmt), oe1->op, oe2->op); @@ -4583,7 +4583,7 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex, unsigned int uid = gimple_uid (stmt); gimple *insert_point = find_insert_point (stmt, new_rhs1, oe->op); - lhs = make_ssa_name (TREE_TYPE (lhs)); + lhs = copy_ssa_name (lhs); stmt = gimple_build_assign (lhs, gimple_assign_rhs_code (stmt), new_rhs1, oe->op); gimple_set_uid (stmt, uid); @@ -4820,7 +4820,7 @@ linearize_expr (gimple *stmt) gsi = gsi_for_stmt (stmt); gimple_assign_set_rhs2 (stmt, gimple_assign_rhs1 (binrhs)); - binrhs = gimple_build_assign (make_ssa_name (TREE_TYPE (lhs)), + binrhs = gimple_build_assign (copy_ssa_name (lhs), gimple_assign_rhs_code (binrhs), gimple_assign_lhs (binlhs), gimple_assign_rhs2 (binrhs)); @@ -4909,7 +4909,7 @@ negate_value (tree tonegate, gimple_stmt_iterator *gsip) rhs2 = negate_value (rhs2, &gsi); gsi = gsi_for_stmt (negatedefstmt); - lhs = make_ssa_name (TREE_TYPE (lhs)); + lhs = copy_ssa_name (lhs); gimple_set_visited (negatedefstmt, true); g = gimple_build_assign (lhs, PLUS_EXPR, rhs1, rhs2); gimple_set_uid (g, gimple_uid (negatedefstmt)); @@ -5245,7 +5245,7 @@ repropagate_negates (void) tree b = gimple_assign_rhs2 (user); gimple_stmt_iterator gsi = gsi_for_stmt (feed); gimple_stmt_iterator gsi2 = gsi_for_stmt (user); - tree x = make_ssa_name (TREE_TYPE (gimple_assign_lhs (feed))); + tree x = copy_ssa_name (gimple_assign_lhs (feed)); gimple *g = gimple_build_assign (x, PLUS_EXPR, a, b); gsi_insert_before (&gsi2, g, GSI_SAME_STMT); gimple_assign_set_rhs_with_ops (&gsi2, NEGATE_EXPR, x); @@ -5737,7 +5737,7 @@ attempt_builtin_copysign (vec *ops) else new_call = gimple_build_call (gimple_call_fndecl (old_call), 2, mul, arg1); - tree lhs = make_ssa_name (type); + tree lhs = copy_ssa_name (oe->op); gimple_call_set_lhs (new_call, lhs); gimple_set_location (new_call, gimple_location (old_call)); @@ -5748,7 +5748,7 @@ attempt_builtin_copysign (vec *ops) /* Handle the CST1 < 0 case by negating the result. */ if (cst1_neg) { - tree negrhs = make_ssa_name (TREE_TYPE (lhs)); + tree negrhs = copy_ssa_name (lhs); gimple *negate_stmt = gimple_build_assign (negrhs, NEGATE_EXPR, lhs); insert_stmt_after (negate_stmt, new_call); @@ -6052,7 +6052,7 @@ reassociate_bb (basic_block bb) if (negate_result) { stmt = SSA_NAME_DEF_STMT (lhs); - tree tmp = make_ssa_name (TREE_TYPE (lhs)); + tree tmp = copy_ssa_name (lhs); gimple_set_lhs (stmt, tmp); if (lhs != new_lhs) tmp = new_lhs;