From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 1BE073858D3C for ; Thu, 25 May 2023 09:19:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1BE073858D3C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B58A51042; Thu, 25 May 2023 02:20:11 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DDE4E3F67D; Thu, 25 May 2023 02:19:25 -0700 (PDT) From: Richard Sandiford To: "Jin Ma" Mail-Followup-To: "Jin Ma" ,"Jeff Law via Gcc-patches" , "Jeff Law" , "jinma.contrib" , richard.sandiford@arm.com Cc: "Jeff Law via Gcc-patches" , "Jeff Law" , "jinma.contrib" Subject: Re: [PATCH] Fix type error of 'switch (SUBREG_BYTE (op)).' References: <20230517090315.795-1-jinma@linux.alibaba.com> Date: Thu, 25 May 2023 10:19:24 +0100 In-Reply-To: (Jin Ma's message of "Thu, 25 May 2023 16:27:03 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-22.5 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: "Jin Ma" 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