From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2e.google.com (mail-oa1-x2e.google.com [IPv6:2001:4860:4864:20::2e]) by sourceware.org (Postfix) with ESMTPS id 7CED93858D1E for ; Wed, 1 Nov 2023 00:05:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7CED93858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7CED93858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:4860:4864:20::2e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698797139; cv=none; b=Qy/hoOI0vjZyoa7Q5CVcKcKAeOSgnMAbOxQ76RKljerBTUrtynYg1QEZpHCQlePdrNekoFKpqDbbomwJ4Hhq31JtYCizLL5ppP7iUREu0fY7c2/+V0BwjawAPIR1yazBU+BUqUXUEBGWy+DNn5U/cf2Ya4f/l4m+Cn1phTxsugM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698797139; c=relaxed/simple; bh=zmZ4vUCqhEnWhE7sFwENHRRGMtjeWvkS+diPuP3VkbE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=RH59t4gOIWAhC1h204IZ6s+mxkfbDcxM0wFG+BQfFtlb0o+43EaEaYaIJkjgJKm09z467GDXRdu3++ymEgOXa/G47WlFHfluFxaIZCdtYatzuCzbTB7RZeEt6b9GqMpPkJ74fv7jiw/S87YvqF++ttMVJeIvO8kK5XiT1fSk3mo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-oa1-x2e.google.com with SMTP id 586e51a60fabf-1ef370c2e12so4450310fac.1 for ; Tue, 31 Oct 2023 17:05:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1698797128; x=1699401928; darn=gcc.gnu.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=vqERuOBUSszHDv6poPwKXe5tH8lCNORnfK/gBdoXk08=; b=kTdwGuTeaK7d5wHw58wZxlZwdjROiYb1uyLX8nUmAmWABVj1ZpMSUVzCkYPxAIT7Si PugxMrLShONAoHCArM4lap6au5N6efDFesI+dtNqq4fHAA20Gexro+3AyxKbKh7gew/i xQLWIGGkIhiBg0LZndgklRx7zKQGEQ4LZlD6K/O8XvUdviQlIYyacQFDTOkCbtNAQb3c rxaALzaILZU/XS/jv8VJozeJdtJywPBAiSAV5gkvL31780c4d2yifAsprfsvqYR+64LQ Cr6qFuU6BbnRh4R3AptJhDarxkUVpjurLb5wOR922wyrucunnbDoVmMlfPWB8afWzSof 9yBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698797128; x=1699401928; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vqERuOBUSszHDv6poPwKXe5tH8lCNORnfK/gBdoXk08=; b=PGTsRq80XEVKokhZsjFwM6ejarzrerfegCWpPCUCLFM5Z59d50+R4eWdtRZjMiwTDl E+ZCwZD9/iYMbroGozuezzm51bLaQOM1dVo/nAIvNlkrQ+FUwC2glo5AerDwgfm6956g hfq89YF6YIwsB43tEhTCllgVarTixJnksnGAqaFDCSBNWBBJoFoA7/4/TbXqZZZEJYK2 4XMIEmMvVeEGEPcZm+Mz9vxoiqffztZY84L90mFcEc8APcFoP6zYSl2FnihrukaCGISe W8SSj/Mmqd7DtROPtdMjwTzGXzHetPOJ3Iq9KWC2p+7MSx+uVBsaefylK9SfBzucsJFp FsXQ== X-Gm-Message-State: AOJu0YwNocY9aIZYhso0PmIshF2uK8RdUuqynbHKnTi86cmgI4AOE1VP PyONqbatdhweHggtgskMlUy8eQ== X-Google-Smtp-Source: AGHT+IH6A2pNQkhPEjTGcw3K/FA/8ofTJQsozNIc2afP5GUpT4Y1+X5j7agzu00tiKz5XfSN9f5fHg== X-Received: by 2002:a05:6870:13ca:b0:1c0:d0e8:8ff9 with SMTP id 10-20020a05687013ca00b001c0d0e88ff9mr13235265oat.16.1698797128681; Tue, 31 Oct 2023 17:05:28 -0700 (PDT) Received: from [10.0.16.165] ([12.44.203.122]) by smtp.gmail.com with ESMTPSA id cr11-20020a056830670b00b006c619f17669sm59049otb.74.2023.10.31.17.05.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 31 Oct 2023 17:05:28 -0700 (PDT) Message-ID: Date: Tue, 31 Oct 2023 17:05:27 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] RISC-V: elide unnecessary sign extend when expanding cmp_and_jump Content-Language: en-US To: Jeff Law , gcc-patches@gcc.gnu.org Cc: gnu-toolchain@rivosinc.com, Robin Dapp , Palmer Dabbelt , Andrew Waterman References: <20231030032122.757369-1-vineetg@rivosinc.com> From: Vineet Gupta In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 10/30/23 13:33, Jeff Law wrote: > >> +/* Helper function for riscv_extend_comparands to Sign-extend the OP. >> +   However if the OP is SI subreg promoted with an inner DI, such as >> +       (subreg/s/v:SI (reg/v:DI) 0 >> +   just peel off the SUBREG to get DI, avoiding extraneous >> extension.  */ >> + >> +static void >> +riscv_sign_extend_if_not_subreg_prom (rtx *op) >> +{ >> +  if (GET_MODE (*op) == SImode So I may have been partially wrong about v2 patch being wrong and needing this fixup ;-) [1] It seems we don't have to limit this to SImode. I re-read the calling convention doc [2] and it says this "When passed in registers or on the stack, integer scalars narrower than XLEN bits are widened according to the sign of their type up to 32 bits, then sign-extended to XLEN bits." This essentially means signed short, and signed char will be already sign-extended at caller site and need not be done in callee: Palmer mention in internal slack that unadorned char is unsigned on RISC-V hence we don't see the compiler extra work for say gcc.dg/torture/pr75964.c. If the test is however tweaked to use signed chars (or short), it seems caller is doing the work (adjusting the constant being passed to be a sign-extended variant). This further validates Jeff's comment about checking for SUBREG_PROMOTED_SIGNED_P (it was anyhow the right thing to begin with anyways). At this point I feel like I'm into splitting hairs (in vain) territory, as fixing this might not matter much in practice .... I'd suppose we go ahead with the v3 with changes Jeff asked for and maybe do a later fixup to relax SI. >> +      && GET_CODE (*op) == SUBREG >> +      && SUBREG_PROMOTED_VAR_P (*op) >> +      && GET_MODE_SIZE (GET_MODE (XEXP (*op, 0))).to_constant () >> +     == GET_MODE_SIZE (word_mode)) >> +    *op = XEXP (*op, 0); >> +  else >> +    *op = gen_rtx_SIGN_EXTEND (word_mode, *op); > So for the wrapped test GET_MODE_SIZE stuff), add parenthesis and > indent the "==" clause.  ie > >   && (GET_MODE_SIZE (GET_MODE (XEXP (*op), 0))).to_constant () >       == GET_MODE_SIZE (word_mode)) > > Don't you also need to verify that the subreg was sign extended? The > PROMOTED_VAR_P just notes that it was promoted, not *how* it was > promoted.  I think you just need to add a test like this: > >   && SUBREG_PROMOTED_SIGNED_P (*op) [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634327.html [2] https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc