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

Reply via email to