On Wed, Jul 1, 2020 at 12:32 PM Eric Botcazou <botca...@adacore.com> wrote:
>
> > + && 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?
>
> OK, added.
>
> > + 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.
>
> Indeed, changed.
>
> > 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.
>
> Will that not interfere with the subsequent cases though? For STRING_CSTs,
> this is not the case since they are not var_decl_component_p. I can replace
> STRING_CST with !var_decl_component_p but I'm not sure what we will gain.
Hmm, that's a good question - so would your patch be replaceable by
simply amending var_decl_component_p by
(var_decl_component_p (TREE_OPERAND (src, 0))
|| TREE_CODE (TREE_OPERAND (src, 0)) == STRING_CST)
?
> > 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?
>
> This would mean implementing the 1) though and, in particular, rejecting or
> splitting long strings, which is precisely what we do not want to do in Ada.
Oh, indeed, yeah - so the separate handling looks better. I thought
of cases like merging "x\0" with a subsequent short integer but those may
be already handled since they are power-of-two sized.
Richard.
> --
> Eric Botcazou
