Re: [i386 PATCH] Improved bit-field assignment RTL expansion

[Roger Sayle <roger at eyesopen dot com>]

Hi Richard,

Thanks for the speedy review.


On Tue, 25 Apr 2006, Richard Henderson wrote:
> Ok, except,
>
> > ! 		      (match_operand 1 "const8_operand" "")
> > ! 		      (match_operand 2 "const8_operand" ""))
> >           (match_operand 3 "register_operand" ""))]
> >     ""
> >   {
> > !   /* Handle insertions to %ah et al.  */
> >     if (INTVAL (operands[1]) != 8 || INTVAL (operands[2]) != 8)
> >       FAIL;
>
> Can we change this to an assert now?

Unfortunately not.  All of these patterns have similar tests in
them, but only insv's comment needed correcting.  It was one of
the things I'd tried, to simply remove these extra tests, but the
bootstrap and regression testing failed with unrecognizable insns.

I think the cause is found in the comments:
  /* From mips.md: extract_bit_field doesn't verify that our source
     matches the predicate, so check it again here.  */
and
  /* From mips.md: insert_bit_field doesn't verify that our source
     matches the predicate, so check it again here.  */

the place where this patch is inserting the test for whether the
"bitsize" predicate matches, isn't actually the place where we
commit to try expanding an insv (or extv).  And in those locations,
we curioursly still continue to ignore the expander's predicates.


If we ever fully honor these predicates, then yes, these lines can
and should be removed.

Roger
--