From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x329.google.com (mail-ot1-x329.google.com [IPv6:2607:f8b0:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id 8BC5A3858C35 for ; Tue, 19 Mar 2024 20:58:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8BC5A3858C35 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sifive.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8BC5A3858C35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::329 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710881939; cv=none; b=LmThyftG7qJ3WNih+kuFaD1rYZtmtqzK1WKUNiR4Xy8Nliw4y+3GWrEjEQLdHFY7QipVcB7X8u9UdT8ejkoEGbqk8KnM+QfhW38SaWi/OjJ3dOEH/+DhBwIzhjMl344Wadr8wEAj6eSlCtnJvIBuOO6RNZx9zpBxmBtezSXbWDs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710881939; c=relaxed/simple; bh=STkZEKUSVHA5s4PtFxsqQ+K3u4W4uJkcAWC4dGBb9AM=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=g6H0a4VTAJFpA9y2ta3xj32RrTLWghci9mbPrhv6h8hidok+wJOKVEypFtT91JgBJTsub4oTtPWwE5jyO3SQMJlbw9TVay5/pXb02rQZheY1FniLnhMBLgFdzpyiYRGogngAzqik3fgbm05bLQDBM9Xhov0aQYRRZJJFMWhZNgA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ot1-x329.google.com with SMTP id 46e09a7af769-6e519d73850so4093652a34.2 for ; Tue, 19 Mar 2024 13:58:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1710881935; x=1711486735; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=rhaEC37FXNADOHxVdRveoaEwVzGL0DuYnQ+9YX2IJGo=; b=eELqnBK2B4R/r4Uhaynsh8Zrjwlxvc4V31wGjO2BApYtsdYB4cUrkXmPyvZGKLrHGo 6szQFFSnuXhDmpjG9DAlKQPkK55EOaJx+d23imwazxRhPcfTTF9CF5DFOdPulU9buSi8 rlJ8uDL5ewLlC0VTiNXMjvVd+xeUTK7tmWIm+Z1JBBNyDR0sp+UV+Nuvz+TQbM3QFLxs hgXeYnf97cTx46zV8F/pjx9QQOf6nOq3MaqNMkZM/BdlNDvJR7MFDYcYhTXINYPKcEDz R16IxCdruOQ191mkW2YJWP9CB6RQTU5XiAQ6mcVc7XLLiz09eYXhRZs+9RvXqd6xcPlQ 4DLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710881935; x=1711486735; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rhaEC37FXNADOHxVdRveoaEwVzGL0DuYnQ+9YX2IJGo=; b=IP4DFo+ISdIKaQD3Mf8ynZaSECtbzWqj87Azm57H38mDDFiSKXaXIeQdMELQWgHMOz DN+K4X8b7isEs2L+K3RGmb9qC2AaXOAod+xJpGhdSuGOUU+b/tZiyqcK558jthzFf+lD DQfFVHmm6nVydo9MdsYHJlMnZRonsQCxvnPqagDM/EMrP/KkwsD102QziJr9Js7GZ+/E TP+QE28LXkz18zK7JSg3KxQIg3uETuKkLKJ7gQX3ZWR8mUh22kl96Xovb7iDczoOFJ+N 13xMgdJUJocLgXGNpxCBVKYVMIVO02pDs5sI1pXQfqy3qcaEBLm9ckuUKL3uA1RVqM7Z y6eQ== X-Forwarded-Encrypted: i=1; AJvYcCVqqW8XLVU5n/2wsi1Hk5m0/iS/tSX7BZKZuQGR+BXth8zXR3A40qpmbjgxA3abrY0vs5LT3idUtMqEjSI/hx5TqE6yT/Ry1Q== X-Gm-Message-State: AOJu0YwU+hATQYjre7nbuIXjui7+XxxDnf/Kcy33Udw2LQtjIBXyt9o+ B5Wi5U3Q9nHUT/HJsDkM6+gD2DpEvcjt5P+m6OlmdYVtbzX2Fm/w23mGQwOA1gS+/bD87tsM6FZ AqbmWShoKLzAGmOWc/gsZAAaV8QCGHPbOCfVLxG2Gam0sBwTzdkj9sBYVTDWdh2WnqEr/mwMAjz PQfaEsHJcU2w4c2zWW+MveU0V1QB7mxB0GRJQ= X-Google-Smtp-Source: AGHT+IFQDXd/9sJ/ZSp8vUOuwZyAauxV2CFHGwDzeuqyWpsRIZUy12RpgohaJxA2Grdg4A/ILCwNIw== X-Received: by 2002:a05:6870:558a:b0:229:7fe1:5674 with SMTP id qj10-20020a056870558a00b002297fe15674mr3234492oac.29.1710881935473; Tue, 19 Mar 2024 13:58:55 -0700 (PDT) Received: from mail-ot1-f53.google.com (mail-ot1-f53.google.com. [209.85.210.53]) by smtp.gmail.com with ESMTPSA id q26-20020a05683022da00b006e6a1879274sm330472otc.25.2024.03.19.13.58.54 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 19 Mar 2024 13:58:54 -0700 (PDT) Received: by mail-ot1-f53.google.com with SMTP id 46e09a7af769-6e67d42422aso2810749a34.0 for ; Tue, 19 Mar 2024 13:58:54 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCUqQD6nlQldiHd4gXjUJflxdk2RvapjVIe5x1FHW8w+nd8HczDrFPp91gEK7ijaH5URieSHWTWLvrHQD55jyYiESk2QDubM7A== X-Received: by 2002:a05:6870:d251:b0:222:3792:d968 with SMTP id h17-20020a056870d25100b002223792d968mr15876817oac.4.1710881933960; Tue, 19 Mar 2024 13:58:53 -0700 (PDT) MIME-Version: 1.0 References: <20240316173524.1147760-1-vineetg@rivosinc.com> <20240316173524.1147760-3-vineetg@rivosinc.com> <78474b71-f605-490d-95d5-9c6a7a162f75@rivosinc.com> <9d5ce8dd-26c3-4bcd-ba35-a44546b64db4@rivosinc.com> In-Reply-To: <9d5ce8dd-26c3-4bcd-ba35-a44546b64db4@rivosinc.com> From: Andrew Waterman Date: Tue, 19 Mar 2024 13:58:42 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [gcc-15 2/3] RISC-V: avoid LUI based const mat: keep stack offsets aligned To: Vineet Gupta Cc: Jeff Law , gcc-patches@gcc.gnu.org, kito.cheng@gmail.com, Palmer Dabbelt , gnu-toolchain@rivosinc.com, Robin Dapp Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3.7 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, Mar 19, 2024 at 1:05=E2=80=AFPM Vineet Gupta = wrote: > > > > On 3/19/24 06:10, Jeff Law wrote: > > On 3/19/24 12:48 AM, Andrew Waterman wrote: > >> On Mon, Mar 18, 2024 at 5:28=E2=80=AFPM Vineet Gupta wrote: > >>> On 3/16/24 13:21, Jeff Law wrote: > >>>> | 59944: add s0,sp,2047 <---- > >>>> | 59948: mv a2,a0 > >>>> | 5994c: mv a3,a1 > >>>> | 59950: mv a0,sp > >>>> | 59954: li a4,1 > >>>> | 59958: lui a1,0x1 > >>>> | 5995c: add s0,s0,1 <--- > >>>> | 59960: jal 59a3c > >>>> > >>>> SP here becomes unaligned, even if transitively which is undesirable= as > >>>> well as incorrect: > >>>> - ABI requires stack to be 8 byte aligned > >>>> - asm code looks weird and unexpected > >>>> - to the user it might falsely seem like a compiler bug even when= not, > >>>> specially when staring at asm for debugging unrelated issue. > >>>> It's not ideal, but I think it's still ABI compliant as-is. If it > >>>> wasn't, then I suspect things like virtual origins in Ada couldn't b= e > >>>> made ABI compliant. > >>> To be clear are u suggesting ADD sp, sp, 2047 is ABI compliant ? > >>> I'd still like to avoid it as I'm sure someone will complain about it= . > >>> > >>>>> With the patch, we get following correct code instead: > >>>>> > >>>>> | .. > >>>>> | 59944: add s0,sp,2032 > >>>>> | .. > >>>>> | 5995c: add s0,s0,16 > >>>> Alternately you could tighten the positive side of the range of the > >>>> splitter from patch 1/3 so that you could always use 2032 rather tha= n > >>>> 2047 on the first addi. ie instead of allowing 2048..4094, allow > >>>> 2048..4064. > >>> 2033..4064 vs. 2048..4094 > >>> > >>> Yeah I was a bit split about this as well. Since you are OK with eith= er, > >>> I'll keep them as-is and perhaps add this observation to commitlog. > >> There's a subset of embedded use cases where an interrupt service > >> routine continues on the same stack as the interrupted thread, > >> requiring sp to always have an ABI-compliant value (i.e. 16B aligned, > >> and with no important data on the stack at an address below sp). > >> > >> Although not all use cases care about this property, it seems more > >> straightforward to maintain the invariant everywhere, rather than > >> selectively enforce it. > > Just to be clear, the changes don't misalign the stack pointer at all. > > They merely have the potential to create *another* pointer into the > > stack which may or may not be aligned. Which is totally normal, it's n= o > > different than taking the address of a char on the stack. > > Right I never saw any sp,sp,2047 getting generated - not even in the > first version of patch which lacked any filtering of stack regs via > riscv_reg_frame_related () and obviously didn't have the stack variant > of splitter. I don't know if that is just being lucky and not enough > testing exposure (I only spot checked buildroot libc, vmlinux) or > something somewhere enforces that. > > However given that misaligned pointer off of stack is a non-issue, I > think we can do the following: > > 1. keep just one splitter with 2047 based predicates and constraint (and > not 2032) for both stack-related and general regs. > 2. gate the splitter on only operands[0] being not stack related > (currently it checks for either [0] or [1]) - this allows the prominent > case where SP is simply a src, and avoids when any potential shenanigans > to SP itself. Agreed. I misread the original code (add s0,sp,2047 looks a lot like add sp,sp,2047 from a quick glance on a cell phone). > > -Vineet