On 11/03/2016 02:00 PM, Jan Hubicka wrote:
>> On 11/03/2016 01:12 PM, Martin Liška wrote:
>>> + tree init = DECL_INITIAL (decl);
>>> + if (init
>>> + && TREE_READONLY (decl)
>>> + && can_convert_ctor_to_string_cst (init))
>>> + DECL_INITIAL (decl) = build_string_cst_from_ctor (init);
>>
>> I'd merge these two new functions since they're only ever called
>> together. We'd then have something like
>>
>> if (init && TREE_READONLY (decl))
>> init = convert_ctor_to_string_cst (init);
>> if (init)
>> DECL_INITIAL (decl) = init;
Done.
>>
>> I'll defer to Jan on whether finalize_decl seems like a good place
>> to do this.
>
> I think finalize_decl may be bit too early because frontends may expects the
> ctors to be in a way they produced them. We only want to convert those arrays
> seen by middle-end so I would move the logic to varpool_node::analyze
>
> Otherwise the patch seems fine to me.
>
> Honza
>>
>>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>>> b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>>> index 283bd1c..b2d1fd5 100644
>>> --- a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
>>> @@ -4,12 +4,15 @@
>>> char *buffer1;
>>> char *buffer2;
>>>
>>> +const char global[] = {'a', 'b', 'c', 'd', '\0'};
>>> +
>>> #define SIZE 1000
>>>
>>> int
>>> main (void)
>>> {
>>> const char* const foo1 = "hello world";
>>> + const char local[] = "abcd";
>>>
>>> buffer1 = __builtin_malloc (SIZE);
>>> __builtin_strcpy (buffer1, foo1);
>>> @@ -45,6 +48,10 @@ main (void)
>>> __builtin_abort ();
>>> if (__builtin_memchr (foo1, null, 12) != foo1 + 11)
>>> __builtin_abort ();
>>> + if (__builtin_memchr (global, null, 5) == 0)
>>> + __builtin_abort ();
>>> + if (__builtin_memchr (local, null, 5) == 0)
>>> + __builtin_abort ();
>>
>> How is that a meaningful test? This seems to work even with an
>> unpatched gcc. I'd have expected something that shows a benefit for
>> doing this conversion, and maybe also a test that shows it isn't
>> done in cases where it's not allowed.
It's meaningful as it scans that there's no __builtin_memchr in optimized dump.
I'm adding new tests that does the opposite test.
>>
>>> tree
>>> -build_string_literal (int len, const char *str)
>>> +build_string_literal (int len, const char *str, bool build_addr_expr)
>>
>> New arguments should be documented in the function comment.
Yep, improved.
>>
>>> +/* Return TRUE when CTOR can be converted to a string constant. */
>>
>> "if", not "when".
Done.
>>
>>> + unsigned HOST_WIDE_INT elements = CONSTRUCTOR_NELTS (ctor);
>>> + FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), idx, key, value)
>>> + {
>>> + if (key == NULL_TREE
>>> + || TREE_CODE (key) != INTEGER_CST
>>> + || !tree_fits_uhwi_p (value)
>>> + || !useless_type_conversion_p (TREE_TYPE (value), char_type_node))
>>> + return false;
>>
>> Shouldn't all elements have the same type, or do you really have to
>> call useless_type_conversion in a loop?
>>
>>> + /* Allow zero character just at the end of a string. */
>>> + if (integer_zerop (value))
>>> + return idx == elements - 1;
>>
>> Don't you also have to explicitly check it's there?
>>
>>
>> Bernd
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Martin
>From 4a2bd43ad10cfb0467dde15ca0be0deba8194ea7 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Mon, 17 Oct 2016 15:24:46 +0200
Subject: [PATCH] Convert character arrays to string csts
gcc/testsuite/ChangeLog:
2016-11-04 Martin Liska <mli...@suse.cz>
* gcc.dg/tree-ssa/builtins-folding-gimple.c (main): Add new
tests.
* gcc.dg/tree-ssa/builtins-folding-gimple-ub.c (main): Likewise.
gcc/ChangeLog:
2016-11-04 Martin Liska <mli...@suse.cz>
* gimplify.c (gimplify_decl_expr): Emit INIT_EXPR just if it
cannot be converted to a string constant.
(gimplify_init_constructor): Create string constant for local
variables (if possible).
* tree.c (convert_ctor_to_string_cst): New function.
(build_string_literal): Add new argument.
(can_convert_ctor_to_string_cst): New function.
* tree.h: Declare new functions.
* varpool.c (ctor_for_folding): Return ctor for local variables.
(varpool_node::analyze): Convert character array ctors to a
string constant (if possible).
---
gcc/gimplify.c | 16 ++++-
.../gcc.dg/tree-ssa/builtins-folding-gimple-ub.c | 20 +++++-
.../gcc.dg/tree-ssa/builtins-folding-gimple.c | 7 ++
gcc/tree.c | 83 ++++++++++++++++++++--
gcc/tree.h | 5 +-
gcc/varpool.c | 14 +++-
6 files changed, 134 insertions(+), 11 deletions(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 1531582..32f0909 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1495,7 +1495,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
{
if (!TREE_STATIC (decl))
{
- DECL_INITIAL (decl) = NULL_TREE;
+ if (!TREE_READONLY (decl) || TREE_CODE (init) != STRING_CST)
+ DECL_INITIAL (decl) = NULL_TREE;
init = build2 (INIT_EXPR, void_type_node, decl, init);
gimplify_and_add (init, seq_p);
ggc_free (init);
@@ -4438,6 +4439,19 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
break;
}
+ /* Replace a ctor with a string constant with possible. */
+ if (TREE_READONLY (object)
+ && VAR_P (object))
+ {
+ tree string_ctor = convert_ctor_to_string_cst (ctor);
+ if (string_ctor)
+ {
+ TREE_OPERAND (*expr_p, 1) = string_ctor;
+ DECL_INITIAL (object) = string_ctor;
+ break;
+ }
+ }
+
/* Fetch information about the constructor to direct later processing.
We might want to make static versions of it in various cases, and
can only do so if it known to be a valid constant initializer. */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
index e1658d1..ea73258 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
@@ -4,11 +4,18 @@
char *buffer1;
char *buffer2;
+const char global1[] = {'a', 'b', 'c', 'd'};
+const char global2[] = {'a', 'b', '\0', 'd', '\0'};
+const char global3[] = {'a', 'b', [3] = 'x', 'c', '\0'};
+const char global4[] = {'a', 'b', 'c', 'd', [5] = '\0'};
+char global5[] = {'a', 'b', 'c', 'd', '\0'};
+
#define SIZE 1000
int
main (void)
{
+ char null = '\0';
const char* const foo1 = "hello world";
/* MEMCHR. */
@@ -17,6 +24,17 @@ main (void)
if (__builtin_memchr (foo1, 'x', 1000)) /* Not folded away. */
__builtin_abort ();
+ if (__builtin_memchr (global1, null, 1) == foo1) /* Not folded away. */
+ __builtin_abort ();
+ if (__builtin_memchr (global2, null, 1) == foo1) /* Not folded away. */
+ __builtin_abort ();
+ if (__builtin_memchr (global3, null, 1) == foo1) /* Not folded away. */
+ __builtin_abort ();
+ if (__builtin_memchr (global4, null, 1) == foo1) /* Not folded away. */
+ __builtin_abort ();
+ if (__builtin_memchr (global5, null, 1) == foo1) /* Not folded away. */
+ __builtin_abort ();
+
/* STRNCMP. */
if (strncmp ("a", "b", -1)) /* { dg-warning "implicit declaration of function" } */
__builtin_abort ();
@@ -24,4 +42,4 @@ main (void)
return 0;
}
-/* { dg-final { scan-tree-dump-times "__builtin_memchr" 2 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__builtin_memchr" 7 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
index 283bd1c..b2d1fd5 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
@@ -4,12 +4,15 @@
char *buffer1;
char *buffer2;
+const char global[] = {'a', 'b', 'c', 'd', '\0'};
+
#define SIZE 1000
int
main (void)
{
const char* const foo1 = "hello world";
+ const char local[] = "abcd";
buffer1 = __builtin_malloc (SIZE);
__builtin_strcpy (buffer1, foo1);
@@ -45,6 +48,10 @@ main (void)
__builtin_abort ();
if (__builtin_memchr (foo1, null, 12) != foo1 + 11)
__builtin_abort ();
+ if (__builtin_memchr (global, null, 5) == 0)
+ __builtin_abort ();
+ if (__builtin_memchr (local, null, 5) == 0)
+ __builtin_abort ();
__builtin_memchr (foo1, x, 11);
__builtin_memchr (buffer1, x, zero);
diff --git a/gcc/tree.c b/gcc/tree.c
index 434aff1..84e5774 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -1784,6 +1784,70 @@ build_vector_from_val (tree vectype, tree sc)
}
}
+/* Return TRUE if CTOR can be converted to a string constant. */
+
+static bool
+can_convert_ctor_to_string_cst (tree ctor)
+{
+ unsigned HOST_WIDE_INT idx;
+ tree value, key;
+
+ tree type = TREE_TYPE (ctor);
+ if (TREE_CODE (ctor) != CONSTRUCTOR
+ || TREE_CODE (type) != ARRAY_TYPE
+ || !tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
+ return false;
+
+ tree subtype = TREE_TYPE (type);
+ if (TYPE_MAIN_VARIANT (subtype) != char_type_node)
+ return false;
+
+ unsigned HOST_WIDE_INT ctor_length = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+
+ /* Skip constructors with a hole. */
+ if (CONSTRUCTOR_NELTS (ctor) != ctor_length)
+ return false;
+
+ FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), idx, key, value)
+ {
+ if (key == NULL_TREE
+ || !tree_fits_uhwi_p (key)
+ || !tree_fits_uhwi_p (value))
+ return false;
+
+ /* Allow zero character only at the end of a string. */
+ if (integer_zerop (value))
+ return idx == ctor_length - 1;
+ else if (!ISPRINT ((char)tree_to_uhwi (value)))
+ return false;
+ }
+
+ return false;
+}
+
+/* Build string constant from a constructor CTOR. */
+
+tree
+convert_ctor_to_string_cst (tree ctor)
+{
+ if (!can_convert_ctor_to_string_cst (ctor))
+ return NULL_TREE;
+
+ unsigned HOST_WIDE_INT idx;
+ tree value;
+ vec<constructor_elt, va_gc> *elts = CONSTRUCTOR_ELTS (ctor);
+ char *str = XNEWVEC (char, elts->length ());
+
+ FOR_EACH_CONSTRUCTOR_VALUE (elts, idx, value)
+ str[idx] = (char)tree_to_uhwi (value);
+
+ tree init = build_string_literal (elts->length (),
+ ggc_alloc_string (str, elts->length ()),
+ false);
+ free (str);
+ return init;
+}
+
/* Something has messed with the elements of CONSTRUCTOR C after it was built;
calculate TREE_CONSTANT and TREE_SIDE_EFFECTS. */
@@ -11359,9 +11423,12 @@ maybe_build_call_expr_loc (location_t loc, combined_fn fn, tree type,
}
/* Create a new constant string literal and return a char* pointer to it.
- The STRING_CST value is the LEN characters at STR. */
+ The STRING_CST value is the LEN characters at STR. If BUILD_ADDR_EXPR
+ is set to true, ADDR_EXPR of the newly created string constant is
+ returned. */
+
tree
-build_string_literal (int len, const char *str)
+build_string_literal (int len, const char *str, bool build_addr_expr)
{
tree t, elem, index, type;
@@ -11374,10 +11441,14 @@ build_string_literal (int len, const char *str)
TREE_READONLY (t) = 1;
TREE_STATIC (t) = 1;
- type = build_pointer_type (elem);
- t = build1 (ADDR_EXPR, type,
- build4 (ARRAY_REF, elem,
- t, integer_zero_node, NULL_TREE, NULL_TREE));
+ if (build_addr_expr)
+ {
+ type = build_pointer_type (elem);
+ t = build1 (ADDR_EXPR, type,
+ build4 (ARRAY_REF, elem,
+ t, integer_zero_node, NULL_TREE, NULL_TREE));
+ }
+
return t;
}
diff --git a/gcc/tree.h b/gcc/tree.h
index 6a98b6e..10ee0d0 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3975,6 +3975,8 @@ extern tree build_vector_stat (tree, tree * MEM_STAT_DECL);
#define build_vector(t,v) build_vector_stat (t, v MEM_STAT_INFO)
extern tree build_vector_from_ctor (tree, vec<constructor_elt, va_gc> *);
extern tree build_vector_from_val (tree, tree);
+extern tree convert_ctor_to_string_cst (tree);
+extern tree build_vector_from_val (tree, tree);
extern void recompute_constructor_flags (tree);
extern void verify_constructor_flags (tree);
extern tree build_constructor (tree, vec<constructor_elt, va_gc> *);
@@ -4022,7 +4024,8 @@ extern tree build_call_expr_internal_loc_array (location_t, enum internal_fn,
tree, int, const tree *);
extern tree maybe_build_call_expr_loc (location_t, combined_fn, tree,
int, ...);
-extern tree build_string_literal (int, const char *);
+extern tree build_string_literal (int len, const char *str,
+ bool build_addr_expr = true);
/* Construct various nodes representing data types. */
diff --git a/gcc/varpool.c b/gcc/varpool.c
index 78969d2..bb16c7b 100644
--- a/gcc/varpool.c
+++ b/gcc/varpool.c
@@ -412,11 +412,15 @@ ctor_for_folding (tree decl)
return error_mark_node;
/* Do not care about automatic variables. Those are never initialized
- anyway, because gimplifier exapnds the code. */
+ anyway, because gimplifier expands the code. */
if (!TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
{
gcc_assert (!TREE_PUBLIC (decl));
- return error_mark_node;
+ if (!TREE_READONLY (decl) || TREE_THIS_VOLATILE (decl))
+ return error_mark_node;
+
+ tree ctor = DECL_INITIAL (decl);
+ return ctor == NULL_TREE ? error_mark_node : ctor;
}
gcc_assert (VAR_P (decl));
@@ -525,6 +529,12 @@ varpool_node::analyze (void)
/* Compute the alignment early so function body expanders are
already informed about increased alignment. */
align_variable (decl, 0);
+
+ tree init = DECL_INITIAL (decl);
+ if (init && TREE_READONLY (decl))
+ init = convert_ctor_to_string_cst (init);
+ if (init)
+ DECL_INITIAL (decl) = init;
}
if (alias)
resolve_alias (varpool_node::get (alias_target));
--
2.10.1
