Re: [PATCH] Fix type error of 'switch (SUBREG_BYTE (op)).'

[Richard Sandiford <richard.sandiford@arm.com>]

Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On 5/17/23 03:03, Jin Ma wrote:
>> For example:
>> (define_insn "mov_lowpart_sidi2"
>>    [(set (match_operand:SI            0 "register_operand" "=r")
>>          (subreg:SI (match_operand:DI 1 "register_operand" " r") 0))]
>>    "TARGET_64BIT"
>>    "mov\t%0,%1")
>> 
>> (define_insn "mov_highpart_sidi2"
>>    [(set (match_operand:SI            0 "register_operand" "=r")
>>          (subreg:SI (match_operand:DI 1 "register_operand" " r") 1))]
>>    "TARGET_64BIT"
>>    "movh\t%0,%1")
>> 
>> When defining the above patterns, the generated file insn-recog.cc will
>> appear 'switch (SUBREG_BYTE (op))', but since the return value of
>> SUBREG_BYTE is poly_uint16_pod, the following error will occur:
>> "error: switch quantity not an integer".
>> 
>> gcc/ChangeLog:
>> 
>> 	* genrecog.cc (print_nonbool_test): Fix type error of
>> 	'switch (SUBREG_BYTE (op))'.
> Thanks.  Installed.

We shouldn't add to_constant just because it's a convenient
way of getting rid of errors :)  There has to be a good reason
in principle why the value is known at compile time.

So I think this should be reverted.  Nothing guarantees that
SUBREG_BYTEs are constant on AArch64 and RISC-V.  And for SVE
it's common for them not to be.

If we want to support the above, I think we need to make the
generator use known_eq instead.

The patterns don't look right though.  An SI subreg of a DI
can't have a SUBREG_BYTE of 1.  And the lowpart SUBREG_BYTE
depends on endianness.  So I think a better way of writing
the lowpart pattern above is to use subreg_lowpart_operator
(which riscv already has).

The high part can't be done using subregs though.

Thanks,
Richard