From patchwork Thu Jun 11 11:52:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 1307501 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=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=adacore-com.20150623.gappssmtp.com header.i=@adacore-com.20150623.gappssmtp.com header.a=rsa-sha256 header.s=20150623 header.b=ZmFqR+1h; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49jMfQ6X0bz9sR4 for ; Thu, 11 Jun 2020 21:52:14 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 01926388E83E; Thu, 11 Jun 2020 11:52:13 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by sourceware.org (Postfix) with ESMTPS id E0732388E83C for ; Thu, 11 Jun 2020 11:52:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E0732388E83C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=botcazou@adacore.com Received: by mail-wr1-x42f.google.com with SMTP id x6so5804426wrm.13 for ; Thu, 11 Jun 2020 04:52:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=NxNqGbtM2tYr7eXrQigMWPaTbfikc2ObZw6kymB4dXM=; b=ZmFqR+1hCvBqoYnVvZhQoh65jciYE+rmW82jj1c3xPWvA/4tqSqoIyrv7IN5CL4rwp A4xW/DuWmIBtJS2CCIQZx0xBiBN2P9i6ccQ7zrQogRwxpLntOduQm7Vp//X5eKrlGYeG Pm3G2d+OJWWnAzLil/HhJJ0oPuEyxSFoe/pAbIXU61g5vN+iUE1oy40x8KbRwnJynRCr p3n7WP50o7gXtaXjbz3gDrKkZgssJIS8uhjnPxEGIlV8w5w2Y1rkx7lBsGlXDJfRYpce NwqQN0n9ikZ/PHe2ToNcAl/ej5NCuL+Ky5eA8uMgcCbvVw8rqjBbB3oC3lRCDNRRrfgV xu4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=NxNqGbtM2tYr7eXrQigMWPaTbfikc2ObZw6kymB4dXM=; b=OoKP+wYmbgocgH1H2Gh57TAVP2nCHRThzstZcuDv++qiqHxkGEFNfMUjwR3nM1JL25 XPcomzxa+SNcX1q+/uokHx3vdwbI0pJrfeYFgdKlBBZh1YrKfm/H+5MKd2qGXrcdUqLI QyHz6cKDLgbNCX1eTuEQg8YLsu14u7aT0fv9atLN/gxurfqFEY2ywG4zW5QSbJQlXGjL ieC1msJij+xR5gxooUDtgMOPvLFJRCFq2Iy0KnhuYGj9Vyy6j3/tfFMayC5TgwWV0A6V BcjnISYCGlV/7SCYjkV3AY/eN3FS/qNejiJB+JgHLFIJelw+jn4u69o9/+rW7A2jW0NV Pgtg== X-Gm-Message-State: AOAM531S7lN5nVP3QQZgIoD4EO7bQfVOS1j2ydXk8nBXqoSAf0Cmo0qH LQoOJ8u/xBZ/GSphXvYzSD0ynz6dZH/Mig== X-Google-Smtp-Source: ABdhPJx9WhISB4S25o4zEzD09Rt+TffbnUth8FzP0fsboKee6rL/xxxXoDw2AMsG9Qboy15RQ3gvmg== X-Received: by 2002:adf:b697:: with SMTP id j23mr9767118wre.201.1591876328475; Thu, 11 Jun 2020 04:52:08 -0700 (PDT) Received: from polaris.localnet ([2a01:e0a:41b:9230:1a03:73ff:fe45:373a]) by smtp.gmail.com with ESMTPSA id x66sm3830938wmb.40.2020.06.11.04.52.07 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Jun 2020 04:52:07 -0700 (PDT) From: Eric Botcazou X-Google-Original-From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: (patch] Optimize assignment to volatile aggregate variable Date: Thu, 11 Jun 2020 13:52:06 +0200 Message-ID: <28946971.1Qo2e1pV5I@polaris> MIME-Version: 1.0 X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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: , Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, gimplify_modify_expr_rhs has an optimization whereby the assignment to an aggregate variable from a read-only object with a DECL_INITIAL is optimized into the direct assignment of the DECL_INITIAL, provided that no temporary is created in the process. The optimization is blocked if the read-only object is volatile, which is OK as per the semantics of volatile, but also if the target variable is volatile, on the grounds that the modified assignment might end up being done on a per field basis, which is also OK. But this latter restriction is enforced a priori and there are cases where the modified assignment would be OK, for example if there is only one field or the DECL_INITIAL is equivalent to the empty CONSTRUCTOR, i.e. all zeros. So, in the latter case, the patch changes gimplify_modify_expr_rhs to ask gimplify_init_constructor whether the assignment would be done as a block, which is easy because gimplify_init_constructor knows that it must create a temporary if the LHS is volatile and this would not be the case, so it's just a matter of completing the NOTIFY_TEMP_CREATION mechanism for this case. Tested on x86-64/Linux, OK for the mainline? 2020-06-11 Eric Botcazou * gimplify.c (gimplify_init_constructor) : Declare new ENSURE_SINGLE_ACCESS constant and move variables down. If it is true and all elements are zero, then always clear. Return GS_ERROR if a temporary would be created for it and NOTIFY_TEMP_CREATION is set. (gimplify_modify_expr_rhs) : If the target is volatile but the type is aggregate non-addressable, ask gimplify_init_constructor whether it can generate a single access to the target. 2020-06-11 Eric Botcazou * gnat.dg/aggr30.adb[sb]: New test. diff --git a/gcc/gimplify.c b/gcc/gimplify.c index e14932fafaf..ff2be7b4fc4 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -4865,10 +4865,6 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, case QUAL_UNION_TYPE: case ARRAY_TYPE: { - struct gimplify_init_ctor_preeval_data preeval_data; - HOST_WIDE_INT num_ctor_elements, num_nonzero_elements; - HOST_WIDE_INT num_unique_nonzero_elements; - bool cleared, complete_p, valid_const_initializer; /* Use readonly data for initializers of this or smaller size regardless of the num_nonzero_elements / num_unique_nonzero_elements ratio. */ @@ -4876,6 +4872,17 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, /* If num_nonzero_elements / num_unique_nonzero_elements ratio is smaller than this, use readonly data. */ const int unique_nonzero_ratio = 8; + /* True if a single access of the object must be ensured. This is the + case if the target is volatile, the type is non-addressable and more + than one field need to be assigned. */ + const bool ensure_single_access + = TREE_THIS_VOLATILE (object) + && !TREE_ADDRESSABLE (type) + && vec_safe_length (elts) > 1; + struct gimplify_init_ctor_preeval_data preeval_data; + HOST_WIDE_INT num_ctor_elements, num_nonzero_elements; + HOST_WIDE_INT num_unique_nonzero_elements; + bool cleared, complete_p, valid_const_initializer; /* Aggregate types must lower constructors to initialization of individual elements. The exception is that a CONSTRUCTOR node @@ -4914,6 +4921,7 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, { if (notify_temp_creation) return GS_ERROR; + DECL_INITIAL (object) = ctor; TREE_STATIC (object) = 1; if (!DECL_NAME (object)) @@ -4961,6 +4969,10 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, /* If there are "lots" of zeros, it's more efficient to clear the memory and then set the nonzero elements. */ cleared = true; + else if (ensure_single_access && num_nonzero_elements == 0) + /* If a single access to the target must be ensured and all elements + are zero, then it's optimal to clear whatever their number. */ + cleared = true; else cleared = false; @@ -5029,13 +5041,14 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, } } - /* If the target is volatile, we have non-zero elements and more than - one field to assign, initialize the target from a temporary. */ - if (TREE_THIS_VOLATILE (object) - && !TREE_ADDRESSABLE (type) - && (num_nonzero_elements > 0 || !cleared) - && vec_safe_length (elts) > 1) + /* If a single access to the target must be ensured and there are + nonzero elements or the zero elements are not assigned en masse, + initialize the target from a temporary. */ + if (ensure_single_access && (num_nonzero_elements > 0 || !cleared)) { + if (notify_temp_creation) + return GS_ERROR; + tree temp = create_tmp_var (TYPE_MAIN_VARIANT (type)); TREE_OPERAND (*expr_p, 0) = temp; *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p), @@ -5243,14 +5256,20 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p, { case VAR_DECL: /* If we're assigning from a read-only variable initialized with - a constructor, do the direct assignment from the constructor, - but only if neither source nor target are volatile since this - latter assignment might end up being done on a per-field basis. */ - if (DECL_INITIAL (*from_p) - && TREE_READONLY (*from_p) + a constructor and not volatile, do the direct assignment from + the constructor, but only if the target is not volatile either + since this latter assignment might end up being done on a per + field basis. However, if the target is volatile and the type + is aggregate and non-addressable, gimplify_init_constructor + knows that it needs to ensure a single access to the target + and it will return GS_OK only in this case. */ + if (TREE_READONLY (*from_p) + && DECL_INITIAL (*from_p) + && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR && !TREE_THIS_VOLATILE (*from_p) - && !TREE_THIS_VOLATILE (*to_p) - && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR) + && (!TREE_THIS_VOLATILE (*to_p) + || (AGGREGATE_TYPE_P (TREE_TYPE (*to_p)) + && !TREE_ADDRESSABLE (TREE_TYPE (*to_p))))) { tree old_from = *from_p; enum gimplify_status subret;