Re: [RFA} Patch to implement "Switch statement fdo reordering"

[Janis Johnson <janis187 at us dot ibm dot com>]

On Tue, 2008-04-29 at 10:47 -0500, Edmar Wienskoski-RA8797 wrote:
> This implements the optimization presented on Gcc Summit 2006:
> http://www.gccsummit.org/2006/view_abstract.php?content_key=18
> 
> At that time Freescale did not have copyright assignment on file.
> Now that we have it, the code was re-implemented over revision 134561,
> and regression tested with x86_64, powerpc64 and powerpc 7450.
> 
> Except for uncovering a latent problem
> (see: http://gcc.gnu.org/ml/gcc/2008-04/msg00614.html)
> there are no regressions.
> 
> Comments / suggestions are appreciated.

Nice, I've wondered when this patch would be submitted!

I haven't looked at the patch itself but have some comments about the
testcase.  First of all, the ChangeLog entry for the test goes into
gcc/testsuite/ChangeLog, and the path will begin with gcc.dg/.

The patch was tested on x86_64 as well as on powerpc (and please
specify the operating system as well as the architecture), but the
single new test is limited to be run on a subset of powerpc targets:

+/* { dg-do run { target powerpc*-*-* } }*/
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-O2 -fdump-tree-expand -mcpu=power4" } */

The requirement for powerpc_altivec_ok is confusing because the test
isn't compiled with -maltivec, and if it were then you'd instead need
to skip the test if it's not running on a machine with AltiVec, not
just a compiler that can generate code for it.  If that check is used
to limit the powerpc targets on which the test will be run to those that
support -mcpu=power4, please add a comment.

If the same test can be used on other targets, the test directives can
be modified to:

 /* { dg-do run { target { { powerpc*-*-* && powerpc_altivec_ok } || { other-targets} } } } */
 /* { dg-options "-O2 -fdump-tree-expand -mcpu=power4" { target powerpc*-*-* } } */
 /* { dg-options "-O2 -fdump-tree-expand -mother1" { target other1*-*-* } } */
 /* { dg-options "-O2 -fdump-tree-expand -mother2" { target other2*-*-* } } */

 /* { dg-final-use { scan-tree-dump "default case 5 with cost 774 promoted" "expand" { target powerpc*-*-* } } */
 /* { dg-final-use { scan-tree-dump "expected string for other1" "expand" { target other1*-*-* } } */
 /* { dg-final-use { scan-tree-dump "expected string for other2" "expand" { target other2*-*-* } } */

Tcl can be picky about spaces around curly braces, so add a space here
between "expand" and "}":

+/* { dg-final-use { scan-tree-dump "default case 5 with cost 774 promoted" "expand"} } */

The test looks fine otherwise.

Janis