From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ciao.gmane.io (ciao.gmane.io [116.202.254.214]) by sourceware.org (Postfix) with ESMTPS id AF3663858D20 for ; Wed, 9 Aug 2023 21:45:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AF3663858D20 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=m.gmane-mx.org Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1qTqzF-0002ZK-SM for gcc-patches@gcc.gnu.org; Wed, 09 Aug 2023 23:45:01 +0200 X-Injected-Via-Gmane: http://gmane.org/ To: gcc-patches@gcc.gnu.org From: Edwin Lu Subject: Re: [PATCH V2] riscv: generate builtin macro for compilation with strict alignment: Date: Wed, 9 Aug 2023 14:40:09 -0700 Message-ID: <5e8de8e3-a1ff-4174-85b4-d67b3c59ac5a@rivosinc.com> References: <20230808202458.3094615-1-ewlu@rivosinc.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit User-Agent: Mozilla Thunderbird Cc: gnu-toolchain@rivosinc.com Content-Language: en-US In-Reply-To: X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Message-ID: <20230809214009.gUZn4qvYPF0474M6WOhi9S3bmEgYIh9WZMLJERYVs6w@z> Hi Vineet, On 8/8/2023 2:02 PM, Vineet Gupta wrote: > > Maybe add a comment that in absence of -m[no-]strict-align, we use the > cpu tune param -> slow_unaligned_access and that default mcpu is rocket > which has that set to _slow. > That sounds good to me! >> +#if defined(__riscv_unaligned_avoid) >> +#error "__riscv_unaligned_avoid is unexpectedly set" >> +#endif > > Lets first check what is really expected. > #if !defined (_slow) #error > >> + >> +/* Check __riscv_unaligned_slow xor __riscv_unaligned_fast is set.  */ >> +#if !defined(__riscv_unaligned_slow) && !defined(__riscv_unaligned_fast) >> +#error "either __riscv_unaligned_slow or__riscv_unaligned_fast is not >> set" >> +#endif >> + >> +#if defined(__riscv_unaligned_slow) && defined(__riscv_unaligned_fast) >> +#error "both __riscv_unaligned_slow and__riscv_unaligned_fast are set" >> +#endif >> + > > I think we can simplify this a bit (sorry I if wasn't clear enough in > our off-list discussions). > We now need to ensure that unexpected toggles are NOT defined: #if > defined(_avoid) || defined(_fast) #error > I don't think we need to check for the combinations as that is covered > by first one and this. > > While separate #error prints for 2 failures are ideal, personally it > feels excessive given that the current implementation will only ever > generate 1 of them. If a future code change breaks that assumption, the > onus is on that change to fix/update the errors. > That makes sense to me. I was thinking that adding some more checks would help clarify the assumptions but it did just make things overly verbose. > > Same as my comment for attribute-1. Please recheck all of them. > Thanks for the feedback! I will be sure to update all of them in the next iteration. Edwin