On Fri, Feb 26, 2016 at 11:02 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> Segher has added last year a few routines for the shift/rotate + mask
> patterns, insns always have one predicate which tests if PowerPC supports
> such pattern, and another that emits the instruction for it.
>
> The testcase in the patch is miscompiled, we end up with an instruction
> with out of bound shift count, because there is disagreement between the
> analysis phase, which does some changes (changes shifts by 0
> into rotate and some rotates into left or right shifts (for the last one
> with changed shift count)), but those changes are only done virtually,
> the predicate can't change the instruction, it either accepts it or rejects
> it.  Then during output, we don't perform those changes, treat it as
> rotate or left or right shift just based on what the actual IL has.
> In some cases it is harmless, in other cases, as the testcase shows, it is
> harmful.
>
> Also, I've noticed that the preparation phase of both
> rs6000_is_valid_shift_mask and rs6000_is_valid_insert_mask is pretty much
> the same (the only difference is that the latter requires the shift/rotate
> count to be always CONST_INT, while the former allows also a REG in there).
>
> So, what the following patch does is that it moves the preparation
> from the predicate functions to a new static inline helper function,
> and then uses that function in both the predicate functions, and also
> in both the output functions.
>
> Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk?
>
> 2016-02-26  Jakub Jelinek  <ja...@redhat.com>
>
>         PR target/69946
>         * config/rs6000/rs6000.c (rs6000_is_valid_shift_mask_1): New
>         function.
>         (rs6000_is_valid_shift_mask, rs6000_is_valid_insert_mask): Use it.
>         (rs6000_insn_for_shift_mask, rs6000_insn_for_insert_mask): Likewise.
>         Adjust for possible changes of shift/rotate CODE and shift count SH.
>
>         * gcc.dg/pr69946.c: New test.

Segher's patch from Wednesday should fix this.

We can address the duplication separately in GCC 7.

Thanks, David

Reply via email to