This is the mail archive of the gcc-bugs@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug target/63679] [5.0 Regression][AArch64] Failure to constant fold.


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63679

--- Comment #15 from jgreenhalgh at gcc dot gnu.org ---
I wonder whether the call to can_move_by_pieces in gimplify.c is bogus. It
seems to me that gimplify.c really wants to know whether it is a good idea to
scalarize the constructor copy - nothing to do with whether we will copy it by
pieces or as a block or otherwise.

If that is what gimplify.c wants to ask, then we can use the SRA parameters I
added last month to get a better answer.

The patch would look something like the below, it won't "fix" this testcase -
but it would allow you to revert to the 4.9 behaviour by tweaking the parameter
value.

It *feels* like the right thing to do, but I don't know the code and I
might be wrong. An alternate approach would be to introduce a new target
hook which returns true if scalarizing a copy is smarter than leaving it
as an aggregate, but that sounds so close to what SRA is supposed to control
as to end up indistinguishable from this patch.

Any thoughts? Or should I just propose this patch on gcc-patches. (It passes an
x86_64 bootstrap with no issues).

---
2014-11-21  James Greenhalgh  <james.greenhalgh@arm.com>

    * gimplify.c (gimplify_init_constructor): Scalarize
    constructor copy based on SRA parameters, rather than
    can_move_by_pieces.
---

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8e3dd83..be51ce7 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -70,6 +70,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "omp-low.h"
 #include "gimple-low.h"
 #include "cilk.h"
+#include "params.h"

 #include "langhooks-def.h"    /* FIXME: for lhd_set_decl_assembler_name */
 #include "tree-pass.h"        /* FIXME: only for PROP_gimple_any */
@@ -3895,7 +3896,6 @@ gimplify_init_constructor (tree *expr_p, gimple_seq
*pre_p, gimple_seq *post_p,
                       DECL_ATTRIBUTES (current_function_decl))))
       {
         HOST_WIDE_INT size = int_size_in_bytes (type);
-        unsigned int align;

         /* ??? We can still get unbounded array types, at least
            from the C++ front end.  This seems wrong, but attempt
@@ -3907,20 +3907,19 @@ gimplify_init_constructor (tree *expr_p, gimple_seq
*pre_p, gimple_seq *post_p,
           TREE_TYPE (ctor) = type = TREE_TYPE (object);
           }

-        /* Find the maximum alignment we can assume for the object.  */
-        /* ??? Make use of DECL_OFFSET_ALIGN.  */
-        if (DECL_P (object))
-          align = DECL_ALIGN (object);
-        else
-          align = TYPE_ALIGN (type);
-
         /* Do a block move either if the size is so small as to make
            each individual move a sub-unit move on average, or if it
-           is so large as to make individual moves inefficient.  */
+           is so large as to make individual moves inefficient.  Reuse
+           the same costs logic as we use in the SRA passes.  */
+            unsigned max_scalarization_size
+          = optimize_function_for_size_p (cfun)
+            ? PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE)
+        : PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED);
+
         if (size > 0
         && num_nonzero_elements > 1
         && (size < num_nonzero_elements
-            || !can_move_by_pieces (size, align)))
+            || size > max_scalarization_size))
           {
         if (notify_temp_creation)
           return GS_ERROR;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]