From 58c3cb725047267b3fac3ec4acee249835b322d6 Mon Sep 17 00:00:00 2001 From: "Darryl L. Miles" Date: Thu, 13 Feb 2025 11:24:40 +0000 Subject: [PATCH 1/4] Compiler support for [[nodiscard]] via __discard__ --- utils/magic.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/utils/magic.h b/utils/magic.h index 7b4356d6..2d075077 100644 --- a/utils/magic.h +++ b/utils/magic.h @@ -204,6 +204,21 @@ extern char AbortMessage[]; /* looking to squash excessive -Wpedantic warnings ? add into defs.mak: CPPFLAGS += -Wno-variadic-macros */ #define ANALYSER_NONNULL(n...) __attribute__((nonnull(n))) #define ANALYSER_RETURNS_NONNULL __attribute__((returns_nonnull)) + + /* These have keyword like behaviour so __nodiscard__ looks more like a keyword and is recognisable as such + * the historic use of __inline__ set a precedent on how backward compatibiliy maybe achieved for such things + */ + +#if (defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 202311L)) + /* since C23 but maybe some compilers supported it before their official C23 releases */ + #define __nodiscard__ [[nodiscard]] +#elif (defined(__GNUC__) && (__GNUC__ >= 4)) + #define __nodiscard__ __attribute__((warn_unused_result)) +#elif (defined(__clang__) && (__clang_major__ >= 1)) + #define __nodiscard__ __attribute__((warn_unused_result)) +#else + #define __nodiscard__ /* */ +#endif #else #define ATTR_FORMAT_PRINTF_1 /* */ #define ATTR_FORMAT_PRINTF_2 /* */ @@ -217,6 +232,8 @@ extern char AbortMessage[]; #define ANALYSER_MALLOC(dealloc, idx) /* */ #define ANALYSER_NONNULL(n...) /* */ #define ANALYSER_RETURNS_NONNULL /* */ + + #define __nodiscard__ /* */ #endif /* ---------------- Start of Machine Configuration Section ----------------- */ From 84203dce53bd5f1bac33bcf0c6988b9367f542a7 Mon Sep 17 00:00:00 2001 From: "Darryl L. Miles" Date: Fri, 31 Jan 2025 20:13:48 +0000 Subject: [PATCH 2/4] CodeQL added __nodiscard__ DBResidueMask() TileTypeBitMask *DBResidueMask(TileType type); /* NOTE: candidate for using a const return */ Added __nodiscard__ to function for compiler assitance. Main problem fixed via another recent comment to remove excessive DBResidueMask() --- database/database.h.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/database.h.in b/database/database.h.in index c5f9f964..d49441d7 100644 --- a/database/database.h.in +++ b/database/database.h.in @@ -900,7 +900,7 @@ extern TileType DBTechFindStacking(); extern bool DBIsContact(); extern PlaneMask DBLayerPlanes(); -extern TileTypeBitMask *DBResidueMask(); +extern __nodiscard__ TileTypeBitMask *DBResidueMask(TileType type); extern void DBFullResidueMask(); /* Miscellaneous */ From d0a6675d456a41b00a4ddf1eab967493c37182d1 Mon Sep 17 00:00:00 2001 From: "Darryl L. Miles" Date: Wed, 19 Feb 2025 11:39:14 +0000 Subject: [PATCH 3/4] makedbh.in: database.h add TTMaskCopy() codegen --- scripts/makedbh.in | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/makedbh.in b/scripts/makedbh.in index 21d6309a..98a8636d 100755 --- a/scripts/makedbh.in +++ b/scripts/makedbh.in @@ -51,6 +51,10 @@ add_generated_mask_macro( "TTMaskZero(m)", "(m)->tt_words[{i}] = 0", ) +add_generated_mask_macro( + "TTMaskCopy(m, n)", + "(m)->tt_words[{i}] = (n)->tt_words[{i}]", +) add_generated_mask_macro( "TTMaskIsZero(m)", "(m)->tt_words[{i}] == 0", From ff6bcecd9fafa0907acb263406d935e03c3d8d85 Mon Sep 17 00:00:00 2001 From: "Darryl L. Miles" Date: Thu, 13 Feb 2025 11:33:16 +0000 Subject: [PATCH 4/4] DBResidueMask() cleanup Reorder things in this function. Adding 'const' in key places to provide the compiler the extra hint for the purpose of this computation we don't change the value and the value never changes externally even across function calls. I'm sure the compiler (due to macro implementation of TTMaskXxxxx() calls and visibility of data being changes) will optimise the function in exactly the way of the reorder. This also should have the side-effect of making clearer more auto vectorization possibilities to the compiler or potentially replacing the loop with (tail-call or inline) to : simd_TTMaskSetMask_residues(lmask, rmask, TT_TECHDEPBASE, DBNumUserLayers); Which would be a hand optimized form, that probably has an 'l_residues' layout that favours SIMD use (2nd order copy from source of truth just in a different data layout, such as contiguous array of TileTypeBitMask indexed from 0, with the TileType as the index). --- database/DBtcontact.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/database/DBtcontact.c b/database/DBtcontact.c index 3a6b1fb0..9a6ef3eb 100644 --- a/database/DBtcontact.c +++ b/database/DBtcontact.c @@ -953,24 +953,22 @@ DBFullResidueMask(type, rmask) TileType type; TileTypeBitMask *rmask; { - TileType t; - TileTypeBitMask *lmask; - LayerInfo *li, *lr; - - li = &dbLayerInfo[type]; - lmask = &li->l_residues; - TTMaskZero(rmask); + LayerInfo *li = &dbLayerInfo[type]; + const TileTypeBitMask *lmask = &li->l_residues; if (type < DBNumUserLayers) { - TTMaskSetMask(rmask, &li->l_residues); + TTMaskCopy(rmask, lmask); } else { - for (t = TT_TECHDEPBASE; t < DBNumUserLayers; t++) + TileType t; + const int tt_last = DBNumUserLayers; + TTMaskZero(rmask); + for (t = TT_TECHDEPBASE; t < tt_last; t++) if (TTMaskHasType(lmask, t)) { - lr = &dbLayerInfo[t]; + LayerInfo *lr = &dbLayerInfo[t]; TTMaskSetMask(rmask, &lr->l_residues); } }