[i386 PATCH] Improved bit-field assignment RTL expansion

Roger Sayle roger@eyesopen.com
Tue Apr 25 22:35:00 GMT 2006


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
--



More information about the Gcc-patches mailing list