On 01/09/2018 02:41 PM, Martin Sebor wrote:
> I found a few problems in the previous revision:
>
> 1) It didn't handle the simple case of member arrays of const
> struct objects (only member arrays of const arrays of structs
> were handled).
> 2) The array_ctor_elt() function returned a narrow empty string
> for an uninitialized CONSTRUCTOR element of any character type
> when it should return the same string in the expected character
> type (char, wchar_t, etc.)
> 3) The string_constant() function would in some cases use a byte
> offset to get the initializer from a CONSTRUCTOR instead of
> an array index.
>
> The attached version 3 of the patch corrects these issues.
> Retested on x86_64 and with the Glibc ToT.
>
>> After sleeping on it I realized that although enhancing
>> gimple_fold_builtin_strlen is an improvement, it only benefits
>> straight calls to strlen and nothing else. Calls to strcmp,
>> sprintf, or strcpy (and along with it the rest of the strlen
>> pass) are still expanded as if the argument were unknown. To
>> improve even those callers, the folding needs to be done at
>> a lower level (otherwise they'd all have to duplicate the same
>> code as gimple_fold_builtin_strlen). With that in mind I've
>> moved the logic to string_constant() so all of those clients
>> benefit.
>>
>> Retested on x86_64-linux. Out of paranoia I also built and
>> tested the top of Glibc trunk with no unusual failures.
>>
>> Martin
>
>
> gcc-83693.diff
>
>
> PR tree-optimization/83693 - missing strlen optimization for array of arrays
>
> gcc/ChangeLog:
>
> PR tree-optimization/83693
> * expr.c (array_ctor_elt): New function.
> (string_constant): Call it. Handle initializers of arrays of arrays.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/83693
> * gcc.dg/strcmp-2.c: New test.
> * gcc.dg/strlenopt-42.c: New test.
> * gcc.dg/strlenopt-43.c: New test.
>
> diff --git a/gcc/expr.c b/gcc/expr.c
> index cd1e57d..75110e5 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -62,7 +62,7 @@ along with GCC; see the file COPYING3. If not see
> #include "rtl-chkp.h"
> #include "ccmp.h"
> #include "rtx-vector-builder.h"
> -
> +#include "gimple-fold.h"
>
> /* If this is nonzero, we do not bother generating VOLATILE
> around volatile memory references, and we are willing to
> @@ -11343,6 +11343,50 @@ is_aligning_offset (const_tree offset, const_tree
> exp)
> return TREE_CODE (offset) == ADDR_EXPR && TREE_OPERAND (offset, 0) == exp;
> }
>
> +/* Return initializer element IDX for the array CONSTRUCTOR initializer
> + INIT or an empty string constant with type CHARTYPE if no such element
> + exists. If IDX is null, simply return an empty string. If IDX is not
> + constant, return NULL_TREE. A helper of string_constant. */
> +
> +static tree
> +array_ctor_elt (tree chartype, tree init, tree idx)
> +{
> + if (idx)
> + {
> + if (!tree_fits_uhwi_p (idx))
> + return NULL_TREE;
> +
> + HOST_WIDE_INT i = tree_to_uhwi (idx);
> +
> + if (i < CONSTRUCTOR_NELTS (init)
> + && tree_int_cst_equal (CONSTRUCTOR_ELT (init, i)->index, idx))
> + return CONSTRUCTOR_ELT (init, i)->value;
> +
> + tree index, value;
> + FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (init), i, index, value)
> + {
> + if (tree_int_cst_equal (index, idx))
> + return value;
> + }
So you first look at IDX and if it patches you return the appropriate value.
Else you iterate from the start of the constructor list through the
elements until you find a match.
Would a binary search between 0..IDX be better here? Do we have any
imposed ordering on the elements?
> + }
> +
> + /* Build and return a STRING_CST representing the empty string with
> + CHARTYPE. Make sure the string representation has enough zero
> + bytes for CHARTYPE. */
> + const char nuls[16] = "";
> + unsigned elemsize = tree_to_uhwi (TYPE_SIZE_UNIT(chartype));
> + tree str = build_string (elemsize, nuls);
> + tree elemtype = build_qualified_type (chartype, TYPE_QUAL_CONST);
> + tree indextype = build_index_type (size_zero_node);
> + tree arraytype = build_array_type (elemtype, indextype);
> + TREE_TYPE (str) = arraytype;
> + TREE_CONSTANT (str) = true;
> + TREE_READONLY (str) = true;
> + TREE_STATIC (str) = true;
I'm a but surprised we don't have a suitable string constant lying
around, but I don't see one.
So really the only concern is the compile-time cost of array_ctor_elt.
If we know anything about the ordering of elements within the
constructor, then we could do a lot better WRT the compile-time cost.
jeff
