[Bug ipa/107640] New: IPA-CP drops known values passed by reference when the reference is to a global variable

[Bug ipa/107640] New: IPA-CP drops known values passed by reference when the reference is to a global variable

[jamborm at gcc dot gnu.org]

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

            Bug ID: 107640
           Summary: IPA-CP drops known values passed by reference when the
                    reference is to a global variable
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jamborm at gcc dot gnu.org
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-linux
            Target: x86_64-linux

Combining IPA-CP of (scalar) parameter values and of values passed by reference
(aggregate values) does not work in the same function parameter, we drop the
aggregate values on the floor.

The following aborts when compiled with -O2 -DFAIL

struct S
{
  int a, b, c;
};

volatile int gi;

void __attribute__((noipa))
consume_s (struct S *p)
{
  gi = p->a;
}

static void __attribute__((noinline))
foo (struct S *p)
{
  if (!__builtin_constant_p (p->b))
    __builtin_abort ();
  consume_s (p);
}

static struct S __attribute((used)) gs;

int main (int argc, char *argv[])
{
  struct S *p;

#ifdef FAIL
  p = &gs;
#else
  struct S s;
  p = &s;
#endif

  p->a = 10;
  p->b = 20;
  foo (p);
  return 0;
}

[Bug ipa/107640] IPA-CP drops known values passed by reference when the reference is to a global variable

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:4834e9360f7bf42fbeabaa20de5619e67c9fee4e

commit r13-4685-g4834e9360f7bf42fbeabaa20de5619e67c9fee4e
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Dec 14 00:33:05 2022 +0100

    ipa: Better way of applying both IPA-CP and IPA-SRA (PR 103227)

    This is basically a better fix for PR 103227.  The one currently in
    use, rushed in late at stage3, which means that IPA-CP transformation
    simply does a replacement of default-definition of IPA-SRA-created
    scalar parameters with a constant, meant that IPA-SRA actually often
    led to creation of a bunch of unused parameters, which was rather
    ironic and sub-optimal.

    This patch rips that old way out and makes sure the clash is resolved
    at clone-materialization time.  What happens is that:

    1) IPA-SRA IPA analysis (decision) stage recognizes the clash and does
       not create a param adjustment entry for such a scalar component.

    2) Clone materialization code checks the IPA-CP transformation
       summary and when it realizes that it is removing a parameter that
       is a base for a discovered IPA-CP aggregate constant, and:

       a) the value is passed by reference, it internally records that any
          load of the value is replaced directly with the known constant
          value.  IPA-SRA will not attempt to split values passed by
          reference when there is a write to it so we know such a load
          won't be on a a LHS.

       b) the value is passed by value, there can be stores to the
          corresponding bit of the aggregate and so all accesses are
          replaced with a new decl and an assignment of the constant to
          this decl is generated at the beginning of the function.

    The new testcase contains an xfail as the patch does not fix PR 107640
    but it is one that ICEs when one is not careful about remapping
    indices of parameters, so I'd like to have it in testsuite/gcc.gd/ipa/
    even now.

    I don't think that PR 107640 should be attempted through
    ipa-param-manipulation replacements because the information is not
    really there any more and we'd either need to do the replacements
    earlier or dig deep into the clone parent info.  Instead, we should
    record somewhere that at the beginning of the function the bits of the
    global decl have known values and use that in the value numbering.
    That way we could one day encode also known constants in globals that
    do not come through parameters.

    gcc/ChangeLog:

    2022-11-11  Martin Jambor  <mjambor@suse.cz>

            PR ipa/103227
            * ipa-param-manipulation.h (class ipa_param_adjustments): Removed
            member function get_updated_index_or_split.
            (class ipa_param_body_adjustments): New overload of
            register_replacement, new member function append_init_stmts, new
            member m_split_agg_csts_inits.
            * ipa-param-manipulation.cc: Include ipa-prop.h.
            (ipa_param_adjustments::get_updated_index_or_split): Removed.
            (ipa_param_body_adjustments::register_replacement): New overload,
use
            it from the older one.
            (ipa_param_body_adjustments::common_initialization): Added the
            capability to create replacements for conflicting IPA-CP discovered
            constants.
            (ipa_param_body_adjustments::ipa_param_body_adjustments): Construct
            the new member.
            (ipa_param_body_adjustments::append_init_stmts): New function.
            * ipa-sra.cc: Include ipa-prop.h.
            (push_param_adjustments_for_index): Require IPA-CP transformation
            summary as a parameter, do not create replacements which are known
to
            have constant values.
            (process_isra_node_results): Find and pass to the above function
the
            IPA-CP transformation summary.
            * ipa-prop.cc (adjust_agg_replacement_values): Remove the
            functionality replacing IPA-SRA created scalar parameters with
            constants.  Simplify, do not require parameter descriptors, do not
            return anything.
            (ipcp_transform_function): Simplify now that
            adjust_agg_replacement_values does not change cfg.  Move definition
            and initialization of descriptors lower.
            * tree-inline.cc (tree_function_versioning): Call append_init_stmts
of
            param_body_adjs, if there are any.

    gcc/testsuite/ChangeLog:

    2022-11-11  Martin Jambor  <mjambor@suse.cz>

            PR ipa/103227
            PR ipa/107640
            * gcc.dg/ipa/pr107640-2.c: New test.