From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa2a.google.com (mail-vk1-xa2a.google.com [IPv6:2607:f8b0:4864:20::a2a]) by sourceware.org (Postfix) with ESMTPS id 549EF3858D20 for ; Tue, 15 Mar 2022 01:04:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 549EF3858D20 Received: by mail-vk1-xa2a.google.com with SMTP id j201so9299893vke.11 for ; Mon, 14 Mar 2022 18:04:34 -0700 (PDT) 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=bzh5MIlI/G+cB0HVI8Jdef/677ELZJqHSALD0HQqT1o=; b=nbHNnx9jSLKH9saXInZd/+76ydIsJ5yS72oSWcAeCMegOGlCwoQM/pP0JwCGaOURnh B+3cNS1KiJUvfGt4oMdkaroVXhhxIfcS+H8mhHYva0wirl5YsT8peRDBeSfTclp813Ol K+ukITlGzgm67YSyYZkEK6ZcJAOXlUovB3PWFV2MwtVsPsYu7gwBbCUjYn7BZFC+WHbI t4Fh/DKlQwRus6iSm/5A6cEpnquiE4zRdLa28M/2+IE78tXle5hge0GUHqMm+bol4VI9 BEbP+xEW3vrJVptjJE/n51zHLnLZ7icCozZhC0+wR1Va6UeixL7tyw3LMM7UzPAokJzU splA== X-Gm-Message-State: AOAM533BegXisFRVUOHHQ2bFDsM5k/WZJ6NsL3uSSOtN6CLeXTomsI7x lxuCOxe1iUEIEAdJNMjCnsNk1oNbPsRjpAdvUdz5Lz8p3ro= X-Google-Smtp-Source: ABdhPJwU+iT/OsmWq0CdbEv5kH+rPsxjTyFyij/kGqcrHjd2eGLvBPgr33IGY/q/3l+vUpNzxwwWuaLiUTy6heaJXRk= X-Received: by 2002:a1f:3244:0:b0:332:2037:83b1 with SMTP id y65-20020a1f3244000000b00332203783b1mr10921795vky.24.1647306273526; Mon, 14 Mar 2022 18:04:33 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Andrew Pinski Date: Mon, 14 Mar 2022 18:04:22 -0700 Message-ID: Subject: Re: RFA: crc builtin functions & optimizations To: Joern Rennecke Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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: 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: Tue, 15 Mar 2022 01:04:36 -0000 On Mon, Mar 14, 2022 at 5:33 PM Joern Rennecke wrote: > > Most microprocessors have efficient ways to perform CRC operations, be > that with lookup tables, rotates, or even special instructions. > However, because we lack a representation for CRC in the compiler, we > can't do proper instruction selection. With this patch I seek out to > rectify this, > I've avoided using a mode name for the built-in functions because that > would tie the semantics to the size of the addressable unit. We > generally use abbreviations like s/l/ll for type names, which is all > right when the type can be widened without changing semantics. For > the data input, however, we also have to consider the shift count that > is tied to it. That is why I used a number to designate the width of > the data input and shift. > > For machine support, I made a start with 8 and 16 bit little-endian > CRC for RISCV using a > lookup table. I am sure once we have the basic infrastructure in the > tree, we'll get more > contributions of suitable named patterns for various ports. A few points. There are at least 9 different polynomials for the CRC-8 in common use today. For CRC-32 there are 5 different polynomials used. You don't have a patch to invoke.texi adding the descriptions of the builtins. How is your polynom 3rd argument described? Is it similar to how it is done on the wiki for the CRC? Does it make sense to have to list the most common polynomials in the documentation? Also I am sorry but micro-optimizing coremarks is just wrong. Maybe it is better to pick the CRC32 that is inside zip instead for a testcase and benchmarking against? Or even the CRC32C for iSCSI/ext4. I see you also don't optimize the case where you have three other variants of polynomials that are reversed, reciprocal and reversed reciocal. Also a huge problem, you don't check to make sure the third argument to the crc builtin function is constant in the rsicv backend. Plus since you expose the crc builtins as a non target specific builtin, I assume there should be a libcall right and therefore either you need to have a generic expansion for it or a function added to libgcc? Also why not expand generically to the table or a loop based on a target hook instead of in the backend? This way a target can choose without even doing much. Thanks, Andrew Pinski > > bootstrapped on x86_64-pc-linux-gnu .