From 5eed6f40aa3635fc0b4a0dfb883399d61967baea Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 23 Feb 2024 14:17:11 -0600 Subject: [PATCH] Changed LFSR_DATA_* macros to consistently create lvalues Most of the LFSR_DATA_* macros were already lvalues due to compound literals: #define LFSR_DATA_BUF(blablabla) \ ((lfsr_data_t){blablabla}) The exception was when an (inlinable) function call was needed, currently LFSR_DATA_CAT, LFSR_DATA_IMM, and LFSR_DATA_LEB128: #define LFSR_DATA_CAT(blablabla) \ lfsr_data_fromcat(blablabla) This gets a bit annoying when you want to pass the result of an LFSR_DATA_* macro by address, sometimes it works, sometimes it doesn't: lfsr_data_size(&LFSR_DATA_BUF(blablabla)); // works lfsr_data_size(&LFSR_DATA_CAT(blablabla)); // doesn't work This may seem like a minor annoyance, but not being able to pass the result of LFSR_DATA_* macros by address becomes a real pain: 1. Most functions accept lfsr_data_t* because it's cheaper. 2. Most of the LFSR_DATA_* macros have compound-literal scope, so creating a temporary requires creating temporaries for all arguments recursively. The solution is to wrap any functions with a compound literal, in this can an array (I tested a struct but it had the same overhead): #define LFSR_DATA_CAT(blablabla) \ ((lfsr_data_t[]){lfsr_data_fromcat(blablabla)}[0]) --- What's really annoying is this introduces a surprising non-zero code-cost: code stack before: 34016 2896 after: 34148 (+0.4%) 2896 (+0.0%) Note this is just adding the above wrappers, not actually using their lvalues yet. I wanted this on a separate commit because the added code-cost is surprising. Unless I'm missing something, the semantics haven't changed, so in theory a perfect compiler should optimize away any in-stack moves? I'm not sure why it fails here. I don't know how I feel about changing code just because of a compiler idiosyncrasy. So I'm going to keep this for now. At some point in the future, it may be worth considering alternatives to our use of compound literals if they really interact with compiler optimizations so poorly... --- lfs.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lfs.c b/lfs.c index f110f65a..0142ec67 100644 --- a/lfs.c +++ b/lfs.c @@ -1115,16 +1115,20 @@ static lfs_ssize_t lfsr_bd_progtag(lfs_t *lfs, .u.buf.buffer=(const void*)(_buffer)}) #define LFSR_DATA_IMM(_buffer, _size) \ - lfsr_data_fromimm(_buffer, _size) + ((const lfsr_data_t[]){ \ + lfsr_data_fromimm(_buffer, _size)}[0]) #define LFSR_DATA_LEB128(_word) \ - lfsr_data_fromleb128(_word) + ((const lfsr_data_t[]){ \ + lfsr_data_fromleb128(_word)}[0]) // this relies on temporary allocations which is a bit precarious... #define LFSR_DATA_CAT(...) \ - lfsr_data_fromcat( \ - (const lfsr_data_t[]){__VA_ARGS__}, \ - sizeof((const lfsr_data_t[]){__VA_ARGS__}) / sizeof(lfsr_data_t)) + ((const lfsr_data_t[]){ \ + lfsr_data_fromcat( \ + (const lfsr_data_t[]){__VA_ARGS__}, \ + sizeof((const lfsr_data_t[]){__VA_ARGS__}) \ + / sizeof(lfsr_data_t))}[0]) // These aren't true runtime-typed datas, but allows some special cases to // bypass data encoding. External context is required to access these