Lulu Cheng <chengl...@loongson.cn> writes:
> 在 2025/1/22 上午8:49, Xi Ruoyao 写道:
>> The second source register of this insn cannot be the same as the
>> destination register.
>>
>> gcc/ChangeLog:
>>
>> * config/loongarch/loongarch.md
>> (<optab>_alsl_reversesi_extended): Add '&' to the destination
>> register constraint and append '0' to the first source register
>> constraint to indicate the destination register cannot be same
>> as the second source register, and change the split condition to
>> reload_completed so that the insn will be split only after RA in
>> order to obtain allocated registers that satisfy the above
>> constraints.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/loongarch/bitwise-shift-reassoc-clobber.c: New
>> test.
>> ---
>> gcc/config/loongarch/loongarch.md | 6 +++---
>> .../loongarch/bitwise-shift-reassoc-clobber.c | 21 +++++++++++++++++++
>> 2 files changed, 24 insertions(+), 3 deletions(-)
>> create mode 100644
>> gcc/testsuite/gcc.target/loongarch/bitwise-shift-reassoc-clobber.c
>>
>> diff --git a/gcc/config/loongarch/loongarch.md
>> b/gcc/config/loongarch/loongarch.md
>> index 223e2b9f37f..1392325038c 100644
>> --- a/gcc/config/loongarch/loongarch.md
>> +++ b/gcc/config/loongarch/loongarch.md
>> @@ -3160,13 +3160,13 @@ (define_insn_and_split
>> "<optab>_shift_reverse<X:mode>"
>> ;; add.w => alsl.w, so implement slli.d + and + add.w => and + alsl.w on
>> ;; our own.
>> (define_insn_and_split "<optab>_alsl_reversesi_extended"
>> - [(set (match_operand:DI 0 "register_operand" "=r")
>> + [(set (match_operand:DI 0 "register_operand" "=&r")
>> (sign_extend:DI
>> (plus:SI
>> (subreg:SI
>> (any_bitwise:DI
>> (ashift:DI
>> - (match_operand:DI 1 "register_operand" "r")
>> + (match_operand:DI 1 "register_operand" "r0")
>> (match_operand:SI 2 "const_immalsl_operand" ""))
>> (match_operand:DI 3 "const_int_operand" "i"))
>> 0)
>> @@ -3175,7 +3175,7 @@ (define_insn_and_split
>> "<optab>_alsl_reversesi_extended"
>> && loongarch_reassoc_shift_bitwise (<is_and>, operands[2], operands[3],
>> SImode)"
>> "#"
>> - "&& true"
>> + "&& reload_completed"
>
> I have no problem with this patch.
>
> But, I have always been confused about the use of reload_completed.
>
> I can understand that it needs to be true here, but I don't quite
> understand the following:
>
> ```
>
> (define_insn_and_split "*zero_extendsidi2_internal"
> [(set (match_operand:DI 0 "register_operand" "=r,r,r,r")
> (zero_extend:DI (match_operand:SI 1 "nonimmediate_operand"
> "r,m,ZC,k")))]
> "TARGET_64BIT"
> "@
> bstrpick.d\t%0,%1,31,0
> ld.wu\t%0,%1
> #
> ldx.wu\t%0,%1"
> "&& reload_completed
> && MEM_P (operands[1])
> && (loongarch_14bit_shifted_offset_address_p (XEXP (operands[1], 0),
> SImode)
> && !loongarch_12bit_offset_address_p (XEXP (operands[1], 0),
> SImode))
> && !paradoxical_subreg_p (operands[0])"
> [(set (match_dup 3) (match_dup 1))
> (set (match_dup 0)
> (ior:DI (zero_extend:DI
> (subreg:SI (match_dup 0) 0))
> (match_dup 2)))]
> {
> operands[1] = gen_lowpart (SImode, operands[1]);
> operands[3] = gen_lowpart (SImode, operands[0]);
> operands[2] = const0_rtx;
> }
> [(set_attr "move_type" "arith,load,load,load")
> (set_attr "mode" "DI")])
> ```
>
> What is the role of reload_complete here?
Yeah, I agree it looks odd. In particular, operands[0] should never be
a subreg after RA, so the paradoxical_subreg_p test shouldn't be needed.
And the hard-coded (subreg:SI ... 0) in the expansion pattern doesn't
seem correct for hard registers -- it should be folded down to a single
(reg:SI ...) instead, as for operands[3].
Thanks,
Richard
