From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe33.google.com (mail-vs1-xe33.google.com [IPv6:2607:f8b0:4864:20::e33]) by sourceware.org (Postfix) with ESMTPS id 9E00F3888816 for ; Fri, 18 Mar 2022 07:50:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9E00F3888816 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sifive.com Received: by mail-vs1-xe33.google.com with SMTP id i63so3416983vsi.5 for ; Fri, 18 Mar 2022 00:50:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ZK6UQjVV3roXE1V9Hre8nksmopy41vZgTQVeU9iAwtQ=; b=i24r4OGmfIbIx7l+5j31q+R+ghhNwp4Ez+2H19bnN4Zv2Y8FY+WpvTOo8SzkHKPnmk O0uBYDpCkzpPZnwnkMwWpTaKCyxBmpkTUw5WTx8i6GEu8WZ38AcKYjrAkueldBibJlWz hypmw0CElqyrwjlGmezOtTLLFX99LyThgZZrrKrkEy4708h8YMMVYWfuBhtuX1LLMVKc JVSJad67UktqevYk3CTypN3J1wjYTLbw/VxvvBKDNmSfCYrkOGUCsH/dWaV8M7/787NF Loxhuv9cRtusuDb7tJ5eY80z3nESSOeMeJjoifQBMMuXmrhiYC2e1p58wwOODVU3B6JP cuKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ZK6UQjVV3roXE1V9Hre8nksmopy41vZgTQVeU9iAwtQ=; b=J90mkP3zyGMifT/IgZBESeYBBrSGHsnFC3i4rbhP3/a5r2sUCA349ZZRi95OHc2xfP 7N9Z8/RUtijuhHRlEmdPV3eDa/L00rO9OqXF8ha9qFPoQiwHAzXfdJIVB13q4RKZ76TU 8nhFNP4sIXrIc6M6cBbA3HMwJ2O+cOJbEQ3Kw9axp77hqLECBSMlvhvl7UopbQAFA/sm /ZFb1sB8TUw98HU/GeA/PwKBzDnfRaFN37szSMAz8IusSe4EEPAyNuRnR37WWujkg7BM ymLGzjHIb+bD7a8RpKK9Fsq2zqSqfoUx7K9oKU8vzgJ1C68wwTpVpOOxhLLZBlBcHlgn WgUQ== X-Gm-Message-State: AOAM533bNfPqyt+rMQmbxwjwrIopo45HYUn/S+3tSil4PyhMiCj6TUN4 29NzeiZ1X0esGVAfHPrAEbOOPxHct+CPS44KkefI0vWhNBE= X-Google-Smtp-Source: ABdhPJxSZnl4VJgNBrVsYk/zWO7n9IZbofH0IXlM6+QtGavXSsASRZezcVNGWb93HkznTSryEH8AFm1Vl9rQMTtDnB4= X-Received: by 2002:a05:6102:109b:b0:320:afe5:eca2 with SMTP id s27-20020a056102109b00b00320afe5eca2mr3199200vsr.14.1647589849853; Fri, 18 Mar 2022 00:50:49 -0700 (PDT) MIME-Version: 1.0 References: <01effa16d153dd291c54d1020d30460b45580080.1644373764.git.research_trasio@irq.a4lg.com> <66686bc9-09c0-4b1b-9d06-c51270eb04c2@irq.a4lg.com> In-Reply-To: <66686bc9-09c0-4b1b-9d06-c51270eb04c2@irq.a4lg.com> From: Nelson Chu Date: Fri, 18 Mar 2022 15:50:39 +0800 Message-ID: Subject: Re: [PATCH 4/5] RISC-V: Prefetch hint instructions and operand set To: Tsukasa OI Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Mar 2022 07:50:52 -0000 Hi Tsukasa, Yeah you are right, we should define a new operand for the imm of prefetch instructions, to check if the lower 5 bit is zero. Sorry that I thought wrong before. The patches in the riscv-CMOs-with-paren branch from your github LGTM, so sommited. Thanks Nelson On Fri, Feb 25, 2022 at 3:07 PM Tsukasa OI wrote: > > Hi Nelson, > > I attached new rebased version of my CMO patches (for reference and > NOT an actual submission) so that you can test an example below. > > | (Submission) (Attachment) > | PATCH 1/5 -> PATCH 1-2/2 (splitted and squashed) > | PATCH 4-5/5 -> PATCH 1/2 (squashed and order changed) p > | PATCH 2-3/5 -> PATCH 2/2 (squashed and order changed) m/z > > This reorganization is mainly caused by following open issue: > > I hope this is resolved soon so that I can submit the next version > without extra footnotes. > > > On 2022/02/25 11:50, Nelson Chu wrote: > > Hi Tsukasa, > > > > Reusing the q operand should be enough. Though we may add relocation > > when the offset is a symbol, I think this should be fine. > > > > Consider Christoph patch, > > https://sourceware.org/pipermail/binutils/2022-January/119262.html > > > > His patch is closer to the implementation I will do, just consider the > > compatibility of amo instructions should be perfect. However, you are > > adding more testcases than us. It would be always good to add more > > testcases, so except the f operand, your patches LGTM. > > > > Thanks > > Nelson > > > Unfortunately, simply reusing 'q' doesn't work. > > For overflow checking, 'q' is fine. > > On 2022/02/25 11:50, Nelson Chu wrote:>> case 'q': used_bits |= ENCODE_STYPE_IMM (-1U); break; > >> + case 'f': used_bits |= ENCODE_STYPE_IMM (-1U); break; > > > > I prefer to reuse the q operand here. > > We may use simple fall-through here. > > However, 32-byte alignment constraint requires a new operand type. > > > Let's see... assume that we change to use "q(s)" instead of "f(s)" (you > can just patch the patches I attached). > > # BTW, I use minimal playground to test my GNU Binutils patches > # > # so that this example can be built and linked. > .globl _start > .section .entry, "ax", %progbits > _start: > .option arch,+zicbop > prefetch.i 0(t0) > # Remove Zicbop extension from final ELF > # to disassemble above instruction as `ori', not `prefetch.i'. > .option arch,-zicbop > ret > > If we assemble above example (test.bare.S) and disassemble the result, > it should look like this: > > 0000000080000028 <_start>: > 80000028: 0002e013 ori zero,t0,0 > 8000002c: 8082 ret > > Looks great. But if we change 0(t0) to 1(t0), we DON'T get an error and > the result will look like... > > 0000000080000028 <_start>: > 80000028: 0002e093 ori ra,t0,0 > 8000002c: 8082 ret > > Oh no, a side effect (assignment to ra[x1]) without "illegal operands". > > > It happens because `match_opcode' for prefetch instructions (to test > instruction validity in `riscv_ip') is called before instruction bits > are modified. For store addresses, actual value insertion happens in > `md_apply_fix' function after "successfully assembled" in `riscv_ip'. > > So, we need to test validity of the immediate constant (32-byte aligned) > somewhere. Even if we test it on fixup, it (at least) requires new > BFD relocation type (preventing reuse of case 'q' code path). > > > Of course, if we consider using relocations on prefetch instructions, > we could move, for instance, address insertion. > Still, we absolutely need something new to handle special 32-byte > alignment or we will risk generating corrupted programs. > > > Thanks, > Tsukasa