From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id AF9603858035 for ; Mon, 14 Feb 2022 16:58:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AF9603858035 Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-623-ZMPtOK3hO4uHhCOVfJrTlw-1; Mon, 14 Feb 2022 11:58:32 -0500 X-MC-Unique: ZMPtOK3hO4uHhCOVfJrTlw-1 Received: by mail-qv1-f69.google.com with SMTP id e9-20020a0cf749000000b0042bf697ff6bso12036779qvo.5 for ; Mon, 14 Feb 2022 08:58:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:cc:in-reply-to :content-transfer-encoding; bh=SUp+tlkDyV4iXbNMNRIiLZ0jtqBB3BX7cyC9bajzgmc=; b=ou009Qr0U3u7ryIeSDvMwAsgxY9bW9vmqar9idpeU3k3Qm8ZTPebjlM7hZV7+wMqwT wF/U8WdZU4xxVIOBMDcK03vaO62Fo9Mk/lxjcW1PoZNb/9V56rPNMV6EcTbVMYC2k9K5 yxkzpfjvLoGpEZWIEI3rJ0gXj+LPUrurZLpFp9UpqQkp4tshttAsyfRkf3Xoozs4DRyP vtCX7G2RnPAOQ4eh0bbEYQcoTyThl0jNxow/6yMtSocSzbfTF843F0eQlevGsqhU0bsr OiRgbe/o4eWKNIoegjSW76NfZff2enDbF2kQrZxRKJKhFZNFW/RlzlUJp8Qz40qEc1ef r1yg== X-Gm-Message-State: AOAM531gVTIljvqo3a1Q/2oZOdPyMl1jdxHz2boLR4tDQysSW0nj63Kg vn1iZncgikEQiq4bffG8BA52kBErJlYS+ho2hZ/IRjRrBB0hi1B0qbuNU2Ski1IjtOGQu1z5KId Dm1mJ9Nn50I41pN/aUvua2BhXT4maA38wa9m7mvgrsjIPS5c4R08UP5sm5XhlQR4KqjHkAg== X-Received: by 2002:a05:6214:c42:: with SMTP id r2mr405835qvj.82.1644857909964; Mon, 14 Feb 2022 08:58:29 -0800 (PST) X-Google-Smtp-Source: ABdhPJzwOkLmWSXfw3q46hM9GCDNH4fNnB3YgA2PF2v5AQHPGm4DSgWk8QZIYMaJahB/nTFvn/4jgA== X-Received: by 2002:a05:6214:c42:: with SMTP id r2mr405816qvj.82.1644857909654; Mon, 14 Feb 2022 08:58:29 -0800 (PST) Received: from [192.168.1.113] ([69.165.238.126]) by smtp.gmail.com with ESMTPSA id bp37sm15308399qkb.135.2022.02.14.08.58.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Feb 2022 08:58:29 -0800 (PST) Message-ID: <54dd4562-2ef0-34f0-a99f-56695e3788aa@redhat.com> Date: Mon, 14 Feb 2022 11:58:27 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [pushed] LRA, rs6000, Darwin: Amend lo_sum use for forced constants [PR104117]. To: Vladimir Makarov via Gcc-patches , richard.sandiford@arm.com References: <20220211235904.25765-1-iain@sandoe.co.uk> From: Vladimir Makarov Cc: Iain Sandoe In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Feb 2022 16:58:40 -0000 On 2022-02-14 11:00, Richard Sandiford wrote: > Hi Vlad, > > Vladimir Makarov via Gcc-patches writes: >> >> Hi, Richard.  Change LRA is mine and I approved it for Iain's patch. >> >> I think there is no need for this code and it is misleading.  If >> 'mem[low_sum]' does not work, I don't think that 'reg=low_sum;mem[reg]' >> will help for any existing target.  As machine-dependent code for any >> target most probably (for ppc64 darwin it is exactly the case) checks >> address only in memory, it can wrongly accept wrong address by reloading >> it into reg and use it in memory. So these are my arguments for the >> remove this code from process_address_1. > I'm probably making too much of this, but: > > I think the code is potentially useful in that existing targets do forbid > forbid lo_sum addresses in certain contexts (due to limited offset range) > while still wanting lo_sum to be used to be load the address. If we > handle the high/lo_sum split in generic code then we have more chance > of being able to optimise things. So it feels like this is setting an > unfortunate precedent. > > I still don't understand what went wrong before though (the PR trail > was a bit too long to process :-)). Is there a case where > (lo_sum (high X) X) != X? If so, that seems like a target bug to me. > Or does the target accept (set R1 (lo_sum R2 X)) for an X that cannot > be split into a HIGH/LO_SUM pair? I'd argue that's a target bug too. > Sometimes it is hard to make a line where an RA bug is a bug in machine-dependent code or in RA itself. For this case I would say it is a bug in the both parts. Low-sum is generated by LRA and it does not know that it should be wrapped by unspec for darwin. Generally speaking we could avoid the change in LRA but it would require to do non-trivial analysis in machine dependent code to find cases when 'reg=low_sum ... mem[reg]' is incorrect code for darwin (PIC) target (and may be some other PIC targets too). Therefore I believe the change in LRA is a good solution even if the change can potentially result in less optimized code for some cases.  Taking your concern into account we could probably improve the patch by introducing a hook (I never liked such solutions as we already have too many hooks directing RA) or better to make the LRA change working only for PIC target. Something like this (it probably needs better recognition of pic target): --- a/gcc/lra-constraints.cc +++ b/gcc/lra-constraints.cc @@ -3616,21 +3616,21 @@ process_address_1 (int nop, bool check_only_p,           if (HAVE_lo_sum)             {               /* addr => lo_sum (new_base, addr), case (2) above.  */               insn = emit_insn (gen_rtx_SET                                 (new_reg,                                  gen_rtx_HIGH (Pmode, copy_rtx (addr))));               code = recog_memoized (insn);               if (code >= 0)                 {                   *ad.inner = gen_rtx_LO_SUM (Pmode, new_reg, addr); -                 if (!valid_address_p (op, &ad, cn)) +                 if (!valid_address_p (op, &ad, cn) && !flag_pic)                     {                       /* Try to put lo_sum into register.  */                       insn = emit_insn (gen_rtx_SET                                         (new_reg,                                          gen_rtx_LO_SUM (Pmode, new_reg, addr)));                       code = recog_memoized (insn);                       if (code >= 0)                         {                           *ad.inner = new_reg;                           if (!valid_address_p (op, &ad, cn))