public inbox for gcc-regression@sourceware.org
help / color / mirror / Atom feed
* [TCWG CI] 416.gamess failed to run correctly after gcc: Enable ipa-sra with fnspec attributes
@ 2021-11-15  7:41 ci_notify
  0 siblings, 0 replies; only message in thread
From: ci_notify @ 2021-11-15  7:41 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-regression

After gcc commit ecdf414bd89e6ba251f6b3f494407139b4dbae0e
Author: Jan Hubicka <jh@suse.cz>

    Enable ipa-sra with fnspec attributes

the following benchmarks slowed down by more than 2%:
- 416.gamess failed to run correctly

Below reproducer instructions can be used to re-build both "first_bad" and "last_good" cross-toolchains used in this bisection.  Naturally, the scripts will fail when triggerring benchmarking jobs if you don't have access to Linaro TCWG CI.

For your convenience, we have uploaded tarballs with pre-processed source and assembly files at:
- First_bad save-temps: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/build-ecdf414bd89e6ba251f6b3f494407139b4dbae0e/save-temps/
- Last_good save-temps: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/build-dc777f6b0646fed2f18a580ce249cb6404f89205/save-temps/
- Baseline save-temps: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/build-baseline/save-temps/

Configuration:
- Benchmark: SPEC CPU2006
- Toolchain: GCC + Glibc + GNU Linker
- Version: all components were built from their tip of trunk
- Target: arm-linux-gnueabihf
- Compiler flags: -O3 -flto -marm
- Hardware: NVidia TK1 4x Cortex-A15

This benchmarking CI is work-in-progress, and we welcome feedback and suggestions at linaro-toolchain@lists.linaro.org .  In our improvement plans is to add support for SPEC CPU2017 benchmarks and provide "perf report/annotate" data behind these reports.

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

This commit has regressed these CI configurations:
 - tcwg_bmk_gnu_tk1/gnu-master-arm-spec2k6-O3_LTO

First_bad build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/build-ecdf414bd89e6ba251f6b3f494407139b4dbae0e/
Last_good build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/build-dc777f6b0646fed2f18a580ce249cb6404f89205/
Baseline build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/build-baseline/
Even more details: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/

Reproduce builds:
<cut>
mkdir investigate-gcc-ecdf414bd89e6ba251f6b3f494407139b4dbae0e
cd investigate-gcc-ecdf414bd89e6ba251f6b3f494407139b4dbae0e

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests and test.sh script
mkdir -p artifacts/manifests
curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/manifests/build-baseline.sh --fail
curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/manifests/build-parameters.sh --fail
curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-master-arm-spec2k6-O3_LTO/39/artifact/artifacts/test.sh --fail
chmod +x artifacts/test.sh

# Reproduce the baseline build (build all pre-requisites)
./jenkins-scripts/tcwg_bmk-build.sh @@ artifacts/manifests/build-baseline.sh

# Save baseline build state (which is then restored in artifacts/test.sh)
mkdir -p ./bisect
rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /gcc/ ./ ./bisect/baseline/

cd gcc

# Reproduce first_bad build
git checkout --detach ecdf414bd89e6ba251f6b3f494407139b4dbae0e
../artifacts/test.sh

# Reproduce last_good build
git checkout --detach dc777f6b0646fed2f18a580ce249cb6404f89205
../artifacts/test.sh

cd ..
</cut>

Full commit (up to 1000 lines):
<cut>
commit ecdf414bd89e6ba251f6b3f494407139b4dbae0e
Author: Jan Hubicka <jh@suse.cz>
Date:   Sat Nov 13 12:13:42 2021 +0100

    Enable ipa-sra with fnspec attributes
    
    Enable some ipa-sra on fortran by allowing signature changes on functions
    with "fn spec" attribute when ipa-modref is enabled.  This is possible since ipa-modref
    knows how to preserve things we trace in fnspec and fnspec generated by fortran forntend
    are quite simple and can be analysed automatically now.  To be sure I will also add
    code that merge fnspec to parameters.
    
    This unfortunately hits bug in ipa-param-manipulation when we remove parameter
    that specifies size of variable length parameter. For this reason I added a hack
    that prevent signature changes on such functions and will handle it incrementally.
    
    I tried creating C testcase but it is blocked by another problem that we punt ipa-sra
    on access attribute.  This is optimization regression we ought to fix so I filled
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103223.
    
    As a followup I will add code classifying the type attributes (we have just few) and
    get stats on access attribute.
    
    gcc/ChangeLog:
    
            * ipa-fnsummary.c (compute_fn_summary): Do not give up on signature
            changes on "fn spec" attribute; give up on varadic types.
            * ipa-param-manipulation.c: Include attribs.h.
            (build_adjusted_function_type): New parameter ARG_MODIFIED; if it is
            true remove "fn spec" attribute.
            (ipa_param_adjustments::build_new_function_type): Update.
            (ipa_param_body_adjustments::modify_formal_parameters): update.
            * ipa-sra.c: Include attribs.h.
            (ipa_sra_preliminary_function_checks): Do not check for TYPE_ATTRIBUTES.
---
 gcc/ipa-fnsummary.c          | 36 +++++++++++++++++++++++++++++----
 gcc/ipa-param-manipulation.c | 47 ++++++++++++++++++++++++++++++++++++++++----
 gcc/ipa-sra.c                |  8 +-------
 3 files changed, 76 insertions(+), 15 deletions(-)

diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index 2cfa9a6d0e9..94a80d3ec90 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -3135,10 +3135,38 @@ compute_fn_summary (struct cgraph_node *node, bool early)
        else
 	 info->inlinable = tree_inlinable_function_p (node->decl);
 
-       /* Type attributes can use parameter indices to describe them.  */
-       if (TYPE_ATTRIBUTES (TREE_TYPE (node->decl))
-	   /* Likewise for #pragma omp declare simd functions or functions
-	      with simd attribute.  */
+       bool no_signature = false;
+       /* Type attributes can use parameter indices to describe them.
+	  Special case fn spec since we can safely preserve them in
+	  modref summaries.  */
+       for (tree list = TYPE_ATTRIBUTES (TREE_TYPE (node->decl));
+	    list && !no_signature; list = TREE_CHAIN (list))
+	 if (!flag_ipa_modref
+	     || !is_attribute_p ("fn spec", get_attribute_name (list)))
+	   {
+	     if (dump_file)
+		{
+		  fprintf (dump_file, "No signature change:"
+			   " function type has unhandled attribute %s.\n",
+			   IDENTIFIER_POINTER (get_attribute_name (list)));
+		}
+	     no_signature = true;
+	   }
+       for (tree parm = DECL_ARGUMENTS (node->decl);
+	    parm && !no_signature; parm = DECL_CHAIN (parm))
+	 if (variably_modified_type_p (TREE_TYPE (parm), node->decl))
+	   {
+	     if (dump_file)
+		{
+		  fprintf (dump_file, "No signature change:"
+			   " has parameter with variably modified type.\n");
+		}
+	     no_signature = true;
+	   }
+
+       /* Likewise for #pragma omp declare simd functions or functions
+	  with simd attribute.  */
+       if (no_signature
 	   || lookup_attribute ("omp declare simd",
 				DECL_ATTRIBUTES (node->decl)))
 	 node->can_change_signature = false;
diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index ae3149718ca..991db0d9b1b 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "symtab-clones.h"
 #include "tree-phinodes.h"
 #include "cfgexpand.h"
+#include "attribs.h"
 
 
 /* Actual prefixes of different newly synthetized parameters.  Keep in sync
@@ -281,11 +282,13 @@ fill_vector_of_new_param_types (vec<tree> *new_types, vec<tree> *otypes,
 /* Build and return a function type just like ORIG_TYPE but with parameter
    types given in NEW_PARAM_TYPES - which can be NULL if, but only if,
    ORIG_TYPE itself has NULL TREE_ARG_TYPEs.  If METHOD2FUNC is true, also make
-   it a FUNCTION_TYPE instead of FUNCTION_TYPE.  */
+   it a FUNCTION_TYPE instead of FUNCTION_TYPE.
+   If ARG_MODIFIED is true drop attributes that are no longer up to date.  */
 
 static tree
 build_adjusted_function_type (tree orig_type, vec<tree> *new_param_types,
-			      bool method2func, bool skip_return)
+			      bool method2func, bool skip_return,
+			      bool args_modified)
 {
   tree new_arg_types = NULL;
   if (TYPE_ARG_TYPES (orig_type))
@@ -334,6 +337,17 @@ build_adjusted_function_type (tree orig_type, vec<tree> *new_param_types,
       if (skip_return)
 	TREE_TYPE (new_type) = void_type_node;
     }
+  /* We only support one fn spec attribute on type.  Be sure to remove it.
+     Once we support multiple attributes we will need to be able to unshare
+     the list.  */
+  if (args_modified && TYPE_ATTRIBUTES (new_type))
+    {
+      gcc_checking_assert
+	      (!TREE_CHAIN (TYPE_ATTRIBUTES (new_type))
+	       && (is_attribute_p ("fn spec",
+			  get_attribute_name (TYPE_ATTRIBUTES (new_type)))));
+      TYPE_ATTRIBUTES (new_type) = NULL;
+    }
 
   return new_type;
 }
@@ -460,8 +474,22 @@ ipa_param_adjustments::build_new_function_type (tree old_type,
   else
     new_param_types_p = NULL;
 
+  /* Check if any params type cares about are modified.  In this case will
+     need to drop some type attributes.  */
+  bool modified = false;
+  size_t index = 0;
+  if (m_adj_params)
+    for (tree t = TYPE_ARG_TYPES (old_type);
+	 t && (int)index < m_always_copy_start && !modified;
+	 t = TREE_CHAIN (t), index++)
+      if (index >= m_adj_params->length ()
+	  || get_original_index (index) != (int)index)
+	modified = true;
+
+
   return build_adjusted_function_type (old_type, new_param_types_p,
-				       method2func_p (old_type), m_skip_return);
+				       method2func_p (old_type), m_skip_return,
+				       modified);
 }
 
 /* Build variant of function decl ORIG_DECL which has no return value if
@@ -1467,12 +1495,23 @@ ipa_param_body_adjustments::modify_formal_parameters ()
   if (fndecl_built_in_p (m_fndecl))
     set_decl_built_in_function (m_fndecl, NOT_BUILT_IN, 0);
 
+  bool modified = false;
+  size_t index = 0;
+  if (m_adj_params)
+    for (tree t = TYPE_ARG_TYPES (orig_type);
+	 t && !modified;
+	 t = TREE_CHAIN (t), index++)
+      if (index >= m_adj_params->length ()
+	  || (*m_adj_params)[index].op != IPA_PARAM_OP_COPY
+	  || (*m_adj_params)[index].base_index != index)
+	modified = true;
+
   /* At this point, removing return value is only implemented when going
      through tree_function_versioning, not when modifying function body
      directly.  */
   gcc_assert (!m_adjustments || !m_adjustments->m_skip_return);
   tree new_type = build_adjusted_function_type (orig_type, &m_new_types,
-						m_method2func, false);
+						m_method2func, false, modified);
 
   TREE_TYPE (m_fndecl) = new_type;
   DECL_VIRTUAL_P (m_fndecl) = 0;
diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
index 88036590425..cb0e30507a1 100644
--- a/gcc/ipa-sra.c
+++ b/gcc/ipa-sra.c
@@ -85,6 +85,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-streamer.h"
 #include "internal-fn.h"
 #include "symtab-clones.h"
+#include "attribs.h"
 
 static void ipa_sra_summarize_function (cgraph_node *);
 
@@ -616,13 +617,6 @@ ipa_sra_preliminary_function_checks (cgraph_node *node)
       return false;
     }
 
-  if (TYPE_ATTRIBUTES (TREE_TYPE (node->decl)))
-    {
-      if (dump_file)
-	fprintf (dump_file, "Function type has attributes. \n");
-      return false;
-    }
-
   if (DECL_DISREGARD_INLINE_LIMITS (node->decl))
     {
       if (dump_file)
</cut>
>From hjl@sc.intel.com  Mon Nov 15 09:12:27 2021
Return-Path: <hjl@sc.intel.com>
X-Original-To: gcc-regression@gcc.gnu.org
Delivered-To: gcc-regression@gcc.gnu.org
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by sourceware.org (Postfix) with ESMTPS id A4AE43857C67
 for <gcc-regression@gcc.gnu.org>; Mon, 15 Nov 2021 09:12:26 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A4AE43857C67
X-IronPort-AV: E=McAfee;i="6200,9189,10168"; a="233653045"
X-IronPort-AV: E=Sophos;i="5.87,236,1631602800"; d="scan'208";a="233653045"
Received: from fmsmga006.fm.intel.com ([10.253.24.20])
 by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 15 Nov 2021 01:12:25 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.87,236,1631602800"; d="scan'208";a="734777226"
Received: from scymds02.sc.intel.com ([10.82.73.244])
 by fmsmga006.fm.intel.com with ESMTP; 15 Nov 2021 01:12:25 -0800
Received: from gnu-snb-1.sc.intel.com (gnu-snb-1.sc.intel.com [172.25.33.219])
 by scymds02.sc.intel.com with ESMTP id 1AF9CP8x011893;
 Mon, 15 Nov 2021 01:12:25 -0800
Received: by gnu-snb-1.sc.intel.com (Postfix, from userid 1000)
 id 51072180160; Mon, 15 Nov 2021 01:12:25 -0800 (PST)
Date: Mon, 15 Nov 2021 01:12:25 -0800
To: skpgkp2@gmail.com, hjl.tools@gmail.com, gcc-regression@gcc.gnu.org
Subject: Regressions on master at commit r12-5250 vs commit r12-5239 on
 Linux/i686
User-Agent: Heirloom mailx 12.5 7/5/10
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Message-Id: <20211115091225.51072180160@gnu-snb-1.sc.intel.com>
From: "H.J. Lu" <hjl@sc.intel.com>
X-Spam-Status: No, score=-3468.3 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS,
 KAM_LAZY_DOMAIN_SECURITY, KAM_NUMSUBJECT, SPF_HELO_NONE, SPF_NONE,
 TXREP autolearn=no autolearn_force=no version=3.4.4
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on
 server2.sourceware.org
X-BeenThere: gcc-regression@gcc.gnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Gcc-regression mailing list <gcc-regression.gcc.gnu.org>
List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-regression>,
 <mailto:gcc-regression-request@gcc.gnu.org?subject=unsubscribe>
List-Archive: <https://gcc.gnu.org/pipermail/gcc-regression/>
List-Post: <mailto:gcc-regression@gcc.gnu.org>
List-Help: <mailto:gcc-regression-request@gcc.gnu.org?subject=help>
List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-regression>,
 <mailto:gcc-regression-request@gcc.gnu.org?subject=subscribe>
X-List-Received-Date: Mon, 15 Nov 2021 09:12:28 -0000

New failures:
FAIL: gcc.c-torture/execute/20061220-1.c   -O1  (internal compiler error)
FAIL: gcc.c-torture/execute/20061220-1.c   -O1  (test for excess errors)

New passes:
FAIL: gcc.dg/ipa/ipa-sra-4.c scan-ipa-dump-times sra "Will split parameter" 2


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-11-15  7:41 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15  7:41 [TCWG CI] 416.gamess failed to run correctly after gcc: Enable ipa-sra with fnspec attributes ci_notify

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).