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

["Jin Ma" <jinma@linux.alibaba.com>]

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

I'm trying to understand what you mean. The return value of
the SUBREG_BYTE is poly_uint16_pod. When the value of the
NUM_POLY_INT_COEFFS is 2, using to_constant () to convert
to a constant may lose coeffs[1], right?

I agree with this, we can revert this first, but this problem
always exists, and it seems that we need to find other suitable
ways to solve this error.

Thanks,
Jin