From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id 81AB33858D1E for ; Tue, 19 Mar 2024 20:05:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 81AB33858D1E 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 81AB33858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::62c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710878759; cv=none; b=IZJbruh++AU95M5HzdfJkrxMEcsOUuYbTJF2VYNMzCTONLQcsYKYnKQSQUby85+U7qTjzyJmcJ3rt9OdskboPr1Y+TXzrBhgP455mlVPbzMYwdEUkkBnS0qbZTPOj+ReGcoI1+tjPodB9vaVhpuAdo3mu5wVW76ZbFwRVOr2Q6c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710878759; c=relaxed/simple; bh=3JNnKGMrdhlab6h5liRBTQTJ2dDV7b4Fs+Qf7xwQ62A=; h=DKIM-Signature:Message-ID:Date:MIME-Version:From:Subject:To; b=V4AEHzm9wDqeeJNF4ckAwFjt7pAD9RD173FcLYOmxmJLaA99VYq2BYpvPNt9Rvs39sEvcy19QTlBmzVtL7Jb8sw8vZKvKR/ZSoOnvsffqGasQG7P8a+GChK8d1+jluDfj5qUfyF4gHcdaYh+RUC38dOfWr6ZDukcQfVTZdwbkEk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x62c.google.com with SMTP id d9443c01a7336-1dee5ef2a7bso41651875ad.1 for ; Tue, 19 Mar 2024 13:05:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1710878756; x=1711483556; darn=gcc.gnu.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=PvpQc0n/PRLixrFeWyhPtqT18lpRKFSXME4XetTVfhA=; b=mWptmUvC5CMz+uKIXzpjNqhgdR8pnZ7Bsv3giZwl3QBaeC4eTScvtP547TM8otpcEl KX00aHehQPNF8TYlMLDDpz2yW4dZAeTNJeDUiMOhGE/IO1ERtufVC2rbooMc/pE8g2Fb Z3Qu0DqpTWxZKbDw8duW95ek7vrXIjuc6Xb0F8FClqNuAyKt1iTtZQm/9B70C+R+Es+u 3bVIOrtXnLPE5wsg/fkOXuXvQicAn40h/auJ5IR19r5flxMAajkND2JqvBKtqBy633WK rJFNXzPdlR8HcGMWosa545t4jye5XlpcSaj+QuLVuJHCZPmWs7IgsTn2pm4d914FALmo 5j9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710878756; x=1711483556; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=PvpQc0n/PRLixrFeWyhPtqT18lpRKFSXME4XetTVfhA=; b=ST4d0VI03f/7sN52ejnMjiMcrfZYzjeKodjFv4hEZzbJY0/JOl8pJz8Xgs5jEAbHmi PJMUBVU/9QC4F6xyuehmAqUF7hdK6aPhraGZd9uaIHf3KXKR/NVe+ywARme6o2g64koM l4lt17hpMwOoLUihEVH8OzvOCNj/M4l3bKCzm7l+JhJCskCEgsVCPwORvrhsLPgb+fF9 lCacLSFIEfiyi/dNYsMczDdk8azj7g1+hA9soHlyD8swCo+fBPEY5WyW9RENso9dKwoS Xa2s4F6wkchKAcbYSBx1kelnwowAIBgq3nE/gHFyH5WltXcQjRzd2vWow4xZmqZettEF sv7g== X-Gm-Message-State: AOJu0YyrgBge8D7SG/peO4rvKZeEbqcGb+pjTW36kOkzvqMSMbzztFJ6 TffmnDfgWb2dcYE7YCtB5WcfN2cRcbFEgV9VE3S0HcjGVh7Zg7ISgY0DdN2mHO8= X-Google-Smtp-Source: AGHT+IGPAUNL0bvyQMMCrnWzaCMPC8Vmz/4kgwwgt5l5fHNNeXL27PPw2219Mj9pUo+dN6yEpBr8ZA== X-Received: by 2002:a17:902:ce81:b0:1dd:a8e2:60a4 with SMTP id f1-20020a170902ce8100b001dda8e260a4mr20559899plg.42.1710878756267; Tue, 19 Mar 2024 13:05:56 -0700 (PDT) Received: from [10.0.16.165] ([12.44.203.122]) by smtp.gmail.com with ESMTPSA id h4-20020a170902680400b001dc8d6a9d40sm12076304plk.144.2024.03.19.13.05.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 19 Mar 2024 13:05:55 -0700 (PDT) Message-ID: <9d5ce8dd-26c3-4bcd-ba35-a44546b64db4@rivosinc.com> Date: Tue, 19 Mar 2024 13:05:54 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Vineet Gupta Subject: Re: [gcc-15 2/3] RISC-V: avoid LUI based const mat: keep stack offsets aligned To: Jeff Law , Andrew Waterman Cc: gcc-patches@gcc.gnu.org, kito.cheng@gmail.com, Palmer Dabbelt , gnu-toolchain@rivosinc.com, Robin Dapp References: <20240316173524.1147760-1-vineetg@rivosinc.com> <20240316173524.1147760-3-vineetg@rivosinc.com> <78474b71-f605-490d-95d5-9c6a7a162f75@rivosinc.com> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,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 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 PM 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 be >>>> 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 than >>>> 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 either, >>> 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 no > 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. -Vineet