Hi,
thanks for the guidance. I'm going to post a new version of the patch with the
testcase modified so that it searches for 'return 9;' instead of '= 9;'.
Filip Kastl
On Fri 2023-10-27 13:55:37, Jeff Law wrote:
>
>
> On 10/20/23 07:52, Filip Kastl wrote:
> > On Fri 2023-10-20 15:50:25, Filip Kastl wrote:
> > > Bootstraped and tested* on x86_64-pc-linux-gnu.
> > >
> > > * One testcase (pr79691.c) did regress. However that is because the test
> > > is
> > > dependent on a certain variable not being copy propagated. I will go into
> > > more
> > > detail about this in a reply to this mail.
> >
> > This testcase checks for the string '= 9' being present in the
> > tree-optimized
> > gimple dump ({ dg-final { scan-tree-dump " = 9;" "optimized" } }). This is
> > how
> > the relevant place in the dump looks like without my patch:
> >
> > int f4 (int i)
> > {
> > int _6;
> >
> > <bb 2> [local count: 1073741824]:
> > _6 = 9;
> > return _6;
> >
> > }
> >
> > Note that '= 9' is indeed present but there is an opportunity for copy
> > propagation. With my patch, the copy propagation happens:
> >
> > int f4 (int i)
> > {
> > int _6;
> >
> > <bb 2> [local count: 1073741824]:
> > return 9;
> >
> > }
> >
> > Which means no '= 9' is present and therefore the test fails.
> >
> > What should I do? I don't suppose that changing the testcase to search for
> > just
> > '9' would be wise since the dump may contain other '9's. I could change it
> > to
> > search for 'return 9'. That would make it dependent on some copy propagation
> > being run late enough. However it is currently dependent on *no* copy
> > propagation being run late in the compilation. Also, if the test would
> > search
> > for 'return 9', it would search for the most optimized version of the
> > function
> > f4.
> >
> > Or maybe searching for '9;' would work.
> So in general you have to go back and try to assess the original intent of
> the test. Once you have the original intent, the path forward is often
> clear.
>
> In this specific case the source is:
> +/* Verify -fprintf-return-value results used for constant propagation. */
> +int f4 (int i)
> +{
> + int n1 = __builtin_snprintf (0, 0, "%i", 1234);
> + int n2 = __builtin_snprintf (0, 0, "%i", 12345);
> + return n1 + n2;
> +}
>
> And the intent of the test is to verify that we get constants from the
> snprintf calls and that they in turn simplify to a constant.
>
> That is certainly still the case after your patch, just the form of the
> output is different (the constant is propagated further). So I think
> testing for "return 9" would be the right approach here.
>
> jeff
