From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 09/10] siphash, checksum: Move TBAA explanation to checksum.c
Date: Sat, 23 Sep 2023 00:06:29 +1000 [thread overview]
Message-ID: <20230922140630.3184256-10-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20230922140630.3184256-1-david@gibson.dropbear.id.au>
A number of checksum and hash functions require workarounds for the odd
behaviour of Type-Baased Alias Analysis. We have a detailed comment about
this on siphash_8b() and other functions reference that.
Move the main comment to csume_16b() instead, because we're going to
reorganise things in siphash.c.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
checksum.c | 19 ++++++++++++++-----
siphash.c | 19 +++++--------------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/checksum.c b/checksum.c
index f2b82a4..03b8a7c 100644
--- a/checksum.c
+++ b/checksum.c
@@ -69,8 +69,17 @@
*
* Return: 32-bit sum of 16-bit words
*/
+/* Type-Based Alias Analysis (TBAA) optimisation in gcc 11 and 12 (-flto -O2)
+ * makes these functions essentially useless by allowing reordering of stores of
+ * input data across function calls. Not even declaring @in as char pointer is
+ * enough: disable gcc's interpretation of strict aliasing altogether. See also:
+ *
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106706
+ * https://stackoverflow.com/questions/2958633/gcc-strict-aliasing-and-horror-stories
+ * https://lore.kernel.org/all/alpine.LFD.2.00.0901121128080.6528__33422.5328093909$1232291247$gmane$org@localhost.localdomain/
+ */
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing")))
uint32_t sum_16b(const void *buf, size_t len)
{
const uint16_t *p = buf;
@@ -110,7 +119,7 @@ uint16_t csum_fold(uint32_t sum)
* Return: 16-bit IPv4-style checksum
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init)
{
return (uint16_t)~csum_fold(sum_16b(buf, len) + init);
@@ -247,7 +256,7 @@ void csum_icmp6(struct icmp6hdr *icmp6hr,
* - coding style adaptation
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init)
{
__m256i a, b, sum256, sum_a_hi, sum_a_lo, sum_b_hi, sum_b_lo, c, d;
@@ -395,7 +404,7 @@ less_than_128_bytes:
* Return: 16-bit folded, complemented checksum sum
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
uint16_t csum(const void *buf, size_t len, uint32_t init)
{
return (uint16_t)~csum_fold(csum_avx2(buf, len, init));
@@ -412,7 +421,7 @@ uint16_t csum(const void *buf, size_t len, uint32_t init)
* Return: 16-bit folded, complemented checksum
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
uint16_t csum(const void *buf, size_t len, uint32_t init)
{
return csum_unaligned(buf, len, init);
diff --git a/siphash.c b/siphash.c
index 91bcc5d..d2b068c 100644
--- a/siphash.c
+++ b/siphash.c
@@ -24,17 +24,8 @@
*
* Return: the 64-bit hash output
*/
-/* Type-Based Alias Analysis (TBAA) optimisation in gcc 11 and 12 (-flto -O2)
- * makes these functions essentially useless by allowing reordering of stores of
- * input data across function calls. Not even declaring @in as char pointer is
- * enough: disable gcc's interpretation of strict aliasing altogether. See also:
- *
- * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106706
- * https://stackoverflow.com/questions/2958633/gcc-strict-aliasing-and-horror-stories
- * https://lore.kernel.org/all/alpine.LFD.2.00.0901121128080.6528__33422.5328093909$1232291247$gmane$org@localhost.localdomain/
- */
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing")))
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
/* cppcheck-suppress unusedFunction */
uint64_t siphash_8b(const uint8_t *in, const uint64_t *k)
{
@@ -53,7 +44,7 @@ uint64_t siphash_8b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
/* cppcheck-suppress unusedFunction */
uint64_t siphash_12b(const uint8_t *in, const uint64_t *k)
{
@@ -73,7 +64,7 @@ uint64_t siphash_12b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
{
struct siphash_state state = SIPHASH_INIT(k);
@@ -94,7 +85,7 @@ uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
/* cppcheck-suppress unusedFunction */
uint64_t siphash_32b(const uint8_t *in, const uint64_t *k)
{
@@ -116,7 +107,7 @@ uint64_t siphash_32b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
uint64_t siphash_36b(const uint8_t *in, const uint64_t *k)
{
struct siphash_state state = SIPHASH_INIT(k);
--
@@ -24,17 +24,8 @@
*
* Return: the 64-bit hash output
*/
-/* Type-Based Alias Analysis (TBAA) optimisation in gcc 11 and 12 (-flto -O2)
- * makes these functions essentially useless by allowing reordering of stores of
- * input data across function calls. Not even declaring @in as char pointer is
- * enough: disable gcc's interpretation of strict aliasing altogether. See also:
- *
- * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106706
- * https://stackoverflow.com/questions/2958633/gcc-strict-aliasing-and-horror-stories
- * https://lore.kernel.org/all/alpine.LFD.2.00.0901121128080.6528__33422.5328093909$1232291247$gmane$org@localhost.localdomain/
- */
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing")))
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
/* cppcheck-suppress unusedFunction */
uint64_t siphash_8b(const uint8_t *in, const uint64_t *k)
{
@@ -53,7 +44,7 @@ uint64_t siphash_8b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
/* cppcheck-suppress unusedFunction */
uint64_t siphash_12b(const uint8_t *in, const uint64_t *k)
{
@@ -73,7 +64,7 @@ uint64_t siphash_12b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
{
struct siphash_state state = SIPHASH_INIT(k);
@@ -94,7 +85,7 @@ uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
/* cppcheck-suppress unusedFunction */
uint64_t siphash_32b(const uint8_t *in, const uint64_t *k)
{
@@ -116,7 +107,7 @@ uint64_t siphash_32b(const uint8_t *in, const uint64_t *k)
* Return: the 64-bit hash output
*/
/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing"))) /* See siphash_8b() */
+__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */
uint64_t siphash_36b(const uint8_t *in, const uint64_t *k)
{
struct siphash_state state = SIPHASH_INIT(k);
--
2.41.0
next prev parent reply other threads:[~2023-09-22 14:06 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 14:06 [PATCH 00/10] siphash: cleanups and fixes David Gibson
2023-09-22 14:06 ` [PATCH 01/10] siphash: Make siphash functions consistently return 64-bit results David Gibson
2023-09-22 14:06 ` [PATCH 02/10] siphash: Make sip round calculations an inline function rather than macro David Gibson
2023-09-22 14:06 ` [PATCH 03/10] siphash: Add siphash_feed() helper David Gibson
2023-09-22 14:06 ` [PATCH 04/10] siphash: Clean up hash finalisation with posthash_final() function David Gibson
2023-09-22 14:06 ` [PATCH 05/10] siphash: Fix bug in state initialisation David Gibson
2023-09-22 14:06 ` [PATCH 06/10] siphash: Use more hygienic state initialiser David Gibson
2023-09-27 17:04 ` Stefano Brivio
2023-09-28 1:20 ` David Gibson
2023-09-29 15:19 ` Stefano Brivio
2023-09-22 14:06 ` [PATCH 07/10] siphash: Use specific structure for internal state David Gibson
2023-09-22 14:06 ` [PATCH 08/10] siphash: Make internal helpers public David Gibson
2023-09-22 14:06 ` David Gibson [this message]
2023-09-22 14:06 ` [PATCH 10/10] siphash: Use incremental rather than all-at-once siphash functions David Gibson
2023-09-26 6:23 ` David Gibson
2023-09-26 7:02 ` David Gibson
2023-09-27 17:05 ` Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230922140630.3184256-10-david@gibson.dropbear.id.au \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).