On Fri, 2015-08-28 at 17:56 +0100, Andre Vieira wrote:
[..snip..]
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/forwprop-33.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/forwprop-33.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..984d8b37a01defe0e6852070a7dfa7ace5027c51
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/forwprop-33.c
> @@ -0,0 +1,42 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-forwprop1" } */
> +
> +unsigned short
> +foo (unsigned short a)
> +{
> +  a ^= 0x4002;
> +  a >>= 1;
> +  a |= 0x8000; /* Simplify to ((a >> 1) ^ 0xa001).  */
> +  return a;
> +}
> +
> +unsigned short
> +bar (unsigned short a)
> +{
> +  a |= 0x4002;
> +  a <<= 1;
> +  a ^= 0x0001; /* Simplify to ((a << 1) | 0x8005).  */
> +  return a;
> +}
> +
> +unsigned short
> +baz (unsigned short a)
> +{
> +  a &= 0xd123;
> +  a ^= 0x6040;
> +  a |= 0xc031; /* Simplify to ((a & 0xd123) | 0xe071).  */
> +  return a;
> +}
> +
> +short
> +qux (short a)
> +{
> +  a ^= 0x8002;
> +  a >>= 1;
> +  a |= 0x8000; /* Only move shift inward: (((a >> 1) ^ 0x4001 |) 0x8000).  */
> +  return a;
> +}
> +/* { dg-final { scan-tree-dump "\\^ 40961" "forwprop1" } } */
> +/* { dg-final { scan-tree-dump "\\| 32773" "forwprop1" } } */
> +/* { dg-final { scan-tree-dump "\\| 57457" "forwprop1" } } */
> +/* { dg-final { scan-tree-dump "\\^ -16383" "forwprop1" } } */
> -- 

I can't comment on the patch itself, but I noticed that in the testsuite
addition, you've gathered all the "dg-final" clauses at the end.

I think that this is consistent with existing practice in gcc, but
AFAIK, the dg-final clauses can appear anywhere in the file.

Would it make test cases more readable if we put the dg-final clauses
next to the code that they relate to?  Or, at least after the function
that they relate to?

For example:

  unsigned short
  foo (unsigned short a)
  {
    a ^= 0x4002;
    a >>= 1;
    a |= 0x8000; /* Simplify to ((a >> 1) ^ 0xa001).  */
    return a;
  }
  /* { dg-final { scan-tree-dump "\\^ 40961" "forwprop1" } } */

  unsigned short
  bar (unsigned short a)
  {
      /* snip */ /* Simplify to ((a << 1) | 0x8005).  */
  }
  /* { dg-final { scan-tree-dump "\\| 32773" "forwprop1" } } */

  ... etc ...

I think this may be more readable than the "group them all at the end of
the file approach", especially with testcases with many files and
dg-final directives.

Hope this is constructive and not too "bikesheddy"
Dave

Reply via email to