"Jin Ma" <ji...@linux.alibaba.com> 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 > > 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?
to_constant will abort compilation if coeffs[1] is nonzero. So we should only use it in contexts where coeffs[1] is already known to be zero. We don't know coeffs[1] is already zero here. The purpose of the switch statement is to test the value, which is currently unknown. We handle (subreg ... 0) correctly when all subregs have byte 0. In that case we use an "if" statement and use known_eq to test the SUBREG_BYTE. The problem only occurs when there are multiple possible SUBREG_BYTEs at the same decision point. I agree that it would make sense to support this, probably by emitting a chain of known_eq "if" statements. But before we do that, I think we should check whether there is a legitimate use case. Like I say, the patterns above don't look correct to me. Thanks, Richard
