Re: [RFA][PATCH][pr target/60648] Fix non-canonical RTL from x86 backend -- P1 regression

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Mar 26, 2014 at 11:02:48AM -0600, Jeff Law wrote:
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
> Verified it fixes the original and reduced testcase.

Note, the testcase is missing from your patch.

But I'd question if this is the right place to canonicalize it.
The non-canonical order seems to be created in the generic code, where
do_tablejump does:
11133	  /* ??? The only correct use of CASE_VECTOR_MODE is the one inside the
11134	     GET_MODE_SIZE, because this indicates how large insns are.  The other
11135	     uses should all be Pmode, because they are addresses.  This code
11136	     could fail if addresses and insns are not the same size.  */
11137	  index = gen_rtx_PLUS
11138	    (Pmode,
11139	     gen_rtx_MULT (Pmode, index,
11140			   gen_int_mode (GET_MODE_SIZE (CASE_VECTOR_MODE), Pmode)),
11141	     gen_rtx_LABEL_REF (Pmode, table_label));
and there I don't see why it shouldn't just try to simplify it.
Thus
  index = simplify_gen_binary (MULT, Pmode, index,
			       gen_int_mode (GET_MODE_SIZE (CASE_VECTOR_MODE),
					     Pmode));
  index = simplify_gen_binary (PLUS, Pmode, index,
			       gen_rtx_LABEL_REF (Pmode, table_label));
would be my (untested) preference.  In the usual case where index is
previously a REG (or less frequently a MEM), I guess the simplification
shouldn't make a difference.
Of course it would be better if we optimized this either at the tree level
or during expansion into a normal unconditional jump, but from what I see
we don't have enough info that expand_normal will expand it into a constant
earlier and try_tablejump doesn't get passed labelvec so that it would know
where to jump to.

	Jakub