On 26/02/24 8:37 pm, Peter Bergner wrote:
> On 2/26/24 4:49 AM, Kewen.Lin wrote:
>> on 2024/2/26 14:18, jeevitha wrote:
>>> Hi All,
>>> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
>>> index 6111cc90eb7..e5688ff972a 100644
>>> --- a/gcc/config/rs6000/vsx.md
>>> +++ b/gcc/config/rs6000/vsx.md
>>> @@ -4660,7 +4660,7 @@
>>> (define_expand "vsx_splat_<mode>"
>>> [(set (match_operand:VSX_D 0 "vsx_register_operand")
>>> (vec_duplicate:VSX_D
>>> - (match_operand:<VEC_base> 1 "input_operand")))]
>>> + (match_operand:<VEC_base> 1 "splat_input_operand")))]
>>> "VECTOR_MEM_VSX_P (<MODE>mode)"
>>> {
>>> rtx op1 = operands[1];
>>
>> This hunk actually does force_reg already:
>>
>> ...
>> else if (!REG_P (op1))
>> op1 = force_reg (<VSX_D:VEC_base>mode, op1);
>>
>> but it's assigning to op1 unexpectedly (an omission IMHO), so just
>> simply fix it with:
>>
>> else if (!REG_P (op1))
>> - op1 = force_reg (<VSX_D:VEC_base>mode, op1);
>> + operands[1] = force_reg (<VSX_D:VEC_base>mode, op1);
>
> I agree op1 was an oversight and it should be operands[1].
> That said, I think using more precise predicates is a good thing,
> so I think we should use both Jeevitha's predicate change and
> your operands[1] change.
>
> I'll note that Jeevitha originally had the operands[1] change, but I
> didn't look closely enough at the issue or the pattern and mentioned
> that these kinds of bugs can be caused by too loose constraints and
> predicates, which is when she found the updated predicate to use.
> I believe she already even bootstrapped and regtested the operands[1]
> only change. Jeevitha???
>
>
Yes, Peter. I have already bootstrapped and regtested the operands[1] change.
