On Wed, Jul 1, 2020 at 8:50 AM Eric Botcazou <botca...@adacore.com> wrote:
>
> Hi,
>
> the GIMPLE store merging pass doesn't merge STRING_CSTs in the general case,
> although they are accepted by native_encode_expr; the reason is that the pass
> only works with integral modes, i.e. with chunks whose size is a power of two.
>
> There are two possible ways of extending it to handle STRING_CSTs: 1) lift the
> condition of integral modes and treat STRING_CSTs as other _CST nodes but with
> arbitrary size; 2) implement a specific and separate handling for STRING_CSTs.
>
> The attached patch implements 2) for the following reasons: on the one hand,
> even in Ada where character strings are first-class citizens, cases where
> merging STRING_CSTs with other *_CST nodes would be possible are quite rare in
> practice; on the other hand, string concatenations happen more naturally and
> frequently thanks to the "&" operator, giving rise to merging opportunities.
>
> These opportunites generally occur after inlining, when a strng argument
> passed in a subprogram call is a STRING_CST and is concatenated with other
> STRING_CSTs in the called subprogram. In this case, the concatenation is
> originally expanded by means of a VLA and calls to BUILT_IN_MEMCPY on the
> array, and becomes of fixed length after inlining; in order to avoid having to
> deal with the calls to BUILT_IN_MEMCPY in the GIMPLE store merging pass, the
> patch also adds an optimization to gimple_fold_builtin_memory_op that turns
> calls to BUILT_IN_MEMCPY of STRING_CSTs into direct assignment of STRING_CSTs.
> Unfortunately, doing this in the general case discombobulates the C-oriented
> string-related GIMPLE passes of the optimizer, so it is done only for the
> calls to BUILT_IN_MEMCPY that have been generated during gimplification for
> variable-sized assignments, which are thus marked with a new flag.
>
> Tested on x86-64/Linux, OK for the mainline?
+ && TREE_CODE (TREE_OPERAND (src, 0)) == STRING_CST
+ && tree_int_cst_equal
+ (TYPE_SIZE_UNIT (TREE_TYPE (TREE_OPERAND (src, 0))), len))
+ {
I guess we miss a BITS_PER_UNIT == 8 check here?
+ gimple_set_vuse (new_stmt, gimple_vuse (stmt));
+ gimple_set_vdef (new_stmt, gimple_vdef (stmt));
+ if (gimple_vdef (new_stmt)
+ && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME)
+ SSA_NAME_DEF_STMT (gimple_vdef (new_stmt)) = new_stmt;
you can use gimple_move_vops (new_stmt, stmt) for this.
I wonder if there are objects beside STRING_CSTs that could have their
sizes fixed via inlining, thus, whether you can omit the == STRING_CST
check? That is, I see this change independent of the store-merging
optimization.
Otherwise the memcpy folding part looks OK to me, I skimmed the
store-merging change and didn't find anything suspicious but I wonder
whether not special-casing STRING_CSTs would actually simplify
the code?
Thanks,
Richard.
>
> 2020-07-01 Eric Botcazou <ebotca...@adacore.com>
>
> * gimple-fold.c (gimple_fold_builtin_memory_op): Fold calls that were
> initially created for the assignment of a variable-sized object.
> * gimple-ssa-store-merging.c (struct merged_store_group): Document
> STRING_CST for rhs_code field.
> Add string_concatenation boolean field.
> (merged_store_group::merged_store_group): Initialize it as well as
> bit_insertion here.
> (merged_store_group::do_merge): Set it upon seeing a STRING_CST. Also
> set bit_insertion here upon seeing a BIT_INSERT_EXPR.
> (merged_store_group::apply_stores): Clear it for small regions. Do
> not
> create a power-of-2-sized buffer if it is still true. And do not set
> bit_insertion here again.
> (encode_tree_to_bitpos): Deal with BLKmode for the expression.
> (merged_store_group::can_be_merged_into): Deal with STRING_CST.
> (imm_store_chain_info::coalesce_immediate_stores): Set bit_insertion
> to true after changing MEM_REF stores into BIT_INSERT_EXPR stores.
> (count_multiple_uses): Return 0 for STRING_CST.
> (split_group): Do not split the group for a string concatenation.
> (imm_store_chain_info::output_merged_store): Constify and rename some
> local variables. Build an array type as destination type for a string
> concatenation, as well as a zero mask, and call build_string to build
> the source.
> (lhs_valid_for_store_merging_p): Return true for VIEW_CONVERT_EXPR.
> (pass_store_merging::process_store): Accept STRING_CST on the RHS.
> * gimple.h (gimple_call_alloca_for_var_p): New accessor function.
> * gimplify.c (gimplify_modify_expr_to_memcpy): Set alloca_for_var.
> * tree.h (CALL_ALLOCA_FOR_VAR_P): Document it for BUILT_IN_MEMCPY.
>
>
> 2020-07-01 Eric Botcazou <ebotca...@adacore.com>
>
> * gnat.dg/opt87.adb: New test.
> * gnat.dg/opt87_pkg.ad[sb]: New helper.
>
> --
> Eric Botcazou
