diff --git a/src/cc65/loadexpr.c b/src/cc65/loadexpr.c index 807a5cbe5..394e0a562 100644 --- a/src/cc65/loadexpr.c +++ b/src/cc65/loadexpr.c @@ -33,6 +33,8 @@ +#include + /* cc65 */ #include "codegen.h" #include "error.h" @@ -113,13 +115,23 @@ void LoadExpr (unsigned Flags, struct ExprDesc* Expr) ** field is completely contained in the lower byte, we will throw away ** the high byte anyway and may therefore load just the low byte. */ + unsigned EndBit = 0; /* End bit for bit-fields, or zero if non-bit-field. */ + int AdjustBitField = 0; if (ED_IsBitField (Expr)) { - Flags |= (Expr->BitOffs + Expr->BitWidth <= CHAR_BITS) ? CF_CHAR : CF_INT; + EndBit = Expr->BitOffs + Expr->BitWidth; + AdjustBitField = Expr->BitOffs != 0 || (EndBit != CHAR_BITS && EndBit != INT_BITS); + + Flags |= (EndBit <= CHAR_BITS) ? CF_CHAR : CF_INT; Flags |= CF_UNSIGNED; } else if ((Flags & CF_TYPEMASK) == 0) { Flags |= TypeOf (Expr->Type); } - if (ED_NeedsTest (Expr)) { + + /* Setting CF_TEST will cause the load to perform optimizations and not actually load all + ** bits of the bit-field, instead just computing the condition codes. Therefore, if + ** adjustment is required, we do not set CF_TEST here, but handle it below. + */ + if (ED_NeedsTest (Expr) && !AdjustBitField) { Flags |= CF_TEST; } @@ -188,34 +200,57 @@ void LoadExpr (unsigned Flags, struct ExprDesc* Expr) Internal ("Invalid location in LoadExpr: 0x%04X", ED_GetLoc (Expr)); } - /* Handle bit fields. The actual type may have been casted or - ** converted, so be sure to always use unsigned ints for the - ** operations. + /* Handle bit fields if necessary. The actual type may have been casted or converted, + ** so be sure to always use unsigned ints for the operations. */ - if (ED_IsBitField (Expr)) { + if (AdjustBitField) { /* If the field was loaded as a char, force the shift/mask ops to be char ops. ** If it is a char, the load has already put 0 in the upper byte, so that can remain. ** CF_FORCECHAR does nothing if the type is not CF_CHAR. */ unsigned F = Flags | CF_FORCECHAR | CF_CONST; - /* Shift right by the bit offset */ - g_asr (F, Expr->BitOffs); - - /* Since we have now shifted down, we can do char ops as long as the width fits in - ** a char. + /* We always need to do something with the low byte, so there is no opportunity + ** for optimization by skipping it. */ - if (Expr->BitWidth <= CHAR_BITS) { - F |= CF_CHAR; - } + CHECK (Expr->BitOffs < CHAR_BITS); - /* And by the width if the field doesn't end on a char or int boundary. If it does - ** end on a boundary, then zeros have already been shifted in. g_and emits no code - ** if the mask is all ones. - */ - if (Expr->BitOffs + Expr->BitWidth != CHAR_BITS && - Expr->BitOffs + Expr->BitWidth != INT_BITS) { - g_and (F, (0x0001U << Expr->BitWidth) - 1U); + if (ED_NeedsTest (Expr)) { + /* If we need to do a test, then we avoid shifting (ASR only shifts one bit + ** at a time, so is slow) and just AND with the appropriate mask, then test + ** the result of that. + */ + + /* Avoid overly large shift. */ + if (EndBit == sizeof (unsigned long) * CHAR_BIT) { + g_and (F, (~0UL << Expr->BitOffs)); + } else { + g_and (F, ((1UL << EndBit) - 1) & (~0UL << Expr->BitOffs)); + } + + /* TODO: When long bit-fields are supported, an optimization to test only 3 bytes + ** when EndBit <= 24 is possible. + */ + g_test (F); + ED_TestDone (Expr); + } else { + /* Shift right by the bit offset; no code is emitted if BitOffs is zero */ + g_asr (F, Expr->BitOffs); + + /* Since we have now shifted down, we can do char ops as long as the width fits in + ** a char. + */ + if (Expr->BitWidth <= CHAR_BITS) { + F |= CF_CHAR; + } + + /* And by the width if the field doesn't end on a char or int boundary. + ** If it does end on a boundary, then zeros have already been shifted in. + ** g_and emits no code if the mask is all ones. + */ + if (EndBit != CHAR_BITS && EndBit != INT_BITS) { + g_and (F, (0x0001U << Expr->BitWidth) - 1U); + } } } diff --git a/test/todo/bug1139.c b/test/todo/bug1139.c deleted file mode 100644 index 222a9ab51..000000000 --- a/test/todo/bug1139.c +++ /dev/null @@ -1,52 +0,0 @@ -/* - Copyright 2020 The cc65 Authors - - This software is provided 'as-is', without any express or implied - warranty. In no event will the authors be held liable for any damages - arising from the use of this software. - - Permission is granted to anyone to use this software for any purpose, - including commercial applications, and to alter it and redistribute it - freely, subject to the following restrictions: - - 1. The origin of this software must not be misrepresented; you must not - claim that you wrote the original software. If you use this software - in a product, an acknowledgment in the product documentation would be - appreciated but is not required. - 2. Altered source versions must be plainly marked as such, and must not be - misrepresented as being the original software. - 3. This notice may not be removed or altered from any source distribution. -*/ - -/* - Tests bit-field in if condition; see https://github.com/cc65/cc65/issues/1139 -*/ - -#include - -static unsigned char failures = 0; - -static struct overlap { - unsigned int x : 10; - unsigned int y : 10; -} o = {11, 22}; - -/* Test using bit-fields in if conditions. */ -static void test_if(void) -{ - o.x = 0; - o.y = 44; - if (o.x) { - printf("Bad, o.x is false\n"); - failures++; - } else { - printf("Good\n"); - } -} - -int main(void) -{ - test_if(); - printf("failures: %u\n", failures); - return failures; -} diff --git a/test/val/bug1139.c b/test/val/bug1139.c new file mode 100644 index 000000000..5aef3924d --- /dev/null +++ b/test/val/bug1139.c @@ -0,0 +1,353 @@ +/* + Copyright 2020 The cc65 Authors + + This software is provided 'as-is', without any express or implied + warranty. In no event will the authors be held liable for any damages + arising from the use of this software. + + Permission is granted to anyone to use this software for any purpose, + including commercial applications, and to alter it and redistribute it + freely, subject to the following restrictions: + + 1. The origin of this software must not be misrepresented; you must not + claim that you wrote the original software. If you use this software + in a product, an acknowledgment in the product documentation would be + appreciated but is not required. + 2. Altered source versions must be plainly marked as such, and must not be + misrepresented as being the original software. + 3. This notice may not be removed or altered from any source distribution. +*/ + +/* + Tests bit-field in if condition; see https://github.com/cc65/cc65/issues/1139 +*/ + +#include + +static unsigned char failures = 0; + +static struct four_bits { + unsigned int x : 4; +} fb = {1}; + +static struct overlap { + unsigned int x : 10; + unsigned int y : 10; +} o = {11, 22}; + +static struct full_width { + unsigned int x : 8; + unsigned int y : 16; +} fw = {255, 17}; + +static struct aligned_end { + unsigned int : 2; + unsigned int x : 6; + unsigned int : 3; + unsigned int y : 13; + /* z crosses a byte boundary, but fits in a byte when shifted. */ + unsigned int : 6; + unsigned int z : 7; +} ae = {63, 17, 100}; + +/* Test using bit-fields in if conditions. */ +static void test_if(void) +{ + /* Original test case for the bug. */ + o.x = 0; + o.y = 44; + if (o.x) { + printf("Bad, o.x is false\n"); + failures++; + } else { + printf("Good\n"); + } + + /* Additionally, test most fields from bitfield.c to try to cover all start/end situations. */ + /* four_bits */ + fb.x = 1; + if (fb.x) { + printf("Good\n"); + } else { + printf("Bad, fb.x is true (1)\n"); + failures++; + } + + fb.x = 0; + if (fb.x) { + printf("Bad, fb.x is false\n"); + failures++; + } else { + printf("Good\n"); + } + + /* overlap */ + o.x = 123; + if (o.x) { + printf("Good\n"); + } else { + printf("Bad, o.x is true (123)\n"); + failures++; + } + + o.x = 0; + if (o.x) { + printf("Bad, o.x is false\n"); + failures++; + } else { + printf("Good\n"); + } + + o.y = 321; + if (o.y) { + printf("Good\n"); + } else { + printf("Bad, o.y is true (321)\n"); + failures++; + } + + o.y = 0; + if (o.y) { + printf("Bad, o.y is false\n"); + failures++; + } else { + printf("Good\n"); + } + + /* full_width */ + fw.x = 117; + if (fw.x) { + printf("Good\n"); + } else { + printf("Bad, fw.x is true (117)\n"); + failures++; + } + + fw.x = 0; + if (fw.x) { + printf("Bad, fw.x is false\n"); + failures++; + } else { + printf("Good\n"); + } + + fw.y = 32123; + if (fw.y) { + printf("Good\n"); + } else { + printf("Bad, fw.y is true (32123)\n"); + failures++; + } + + fw.y = 0; + if (fw.y) { + printf("Bad, fw.y is false\n"); + failures++; + } else { + printf("Good\n"); + } + + /* aligned_end */ + ae.x = 2; + if (ae.x) { + printf("Good\n"); + } else { + printf("Bad, ae.x is true (2)\n"); + failures++; + } + + ae.x = 0; + if (ae.x) { + printf("Bad, ae.x is false\n"); + failures++; + } else { + printf("Good\n"); + } + + ae.y = 2222; + if (ae.y) { + printf("Good\n"); + } else { + printf("Bad, ae.y is true (2222)\n"); + failures++; + } + + ae.y = 0; + if (ae.y) { + printf("Bad, ae.y is false\n"); + failures++; + } else { + printf("Good\n"); + } + + ae.z = 111; + if (ae.z) { + printf("Good\n"); + } else { + printf("Bad, ae.z is true (111)\n"); + failures++; + } + + ae.z = 0; + if (ae.z) { + printf("Bad, ae.z is false\n"); + failures++; + } else { + printf("Good\n"); + } +} + +/* Test using bit-fields in inverted if conditions. */ +static void test_if_not(void) +{ + /* Original test case for the bug, inverted. */ + o.x = 0; + o.y = 44; + if (!o.x) { + printf("Good\n"); + } else { + printf("Bad, o.x is false\n"); + failures++; + } + + /* Additionally, test most fields from bitfield.c to try to cover all start/end situations. */ + /* four_bits */ + fb.x = 1; + if (!fb.x) { + printf("Bad, fb.x is true (1)\n"); + failures++; + } else { + printf("Good\n"); + } + + fb.x = 0; + if (!fb.x) { + printf("Good\n"); + } else { + printf("Bad, fb.x is false\n"); + failures++; + } + + /* overlap */ + o.x = 123; + if (!o.x) { + printf("Bad, o.x is true (123)\n"); + failures++; + } else { + printf("Good\n"); + } + + o.x = 0; + if (!o.x) { + printf("Good\n"); + } else { + printf("Bad, o.x is false\n"); + failures++; + } + + o.y = 321; + if (!o.y) { + printf("Bad, o.y is true (321)\n"); + failures++; + } else { + printf("Good\n"); + } + + o.y = 0; + if (!o.y) { + printf("Good\n"); + } else { + printf("Bad, o.y is false\n"); + failures++; + } + + /* full_width */ + fw.x = 117; + if (!fw.x) { + printf("Bad, fw.x is true (117)\n"); + failures++; + } else { + printf("Good\n"); + } + + fw.x = 0; + if (!fw.x) { + printf("Good\n"); + } else { + printf("Bad, fw.x is false\n"); + failures++; + } + + fw.y = 32123; + if (!fw.y) { + printf("Bad, fw.y is true (32123)\n"); + failures++; + } else { + printf("Good\n"); + } + + fw.y = 0; + if (!fw.y) { + printf("Good\n"); + } else { + printf("Bad, fw.y is false\n"); + failures++; + } + + /* aligned_end */ + ae.x = 2; + if (!ae.x) { + printf("Bad, ae.x is true (2)\n"); + failures++; + } else { + printf("Good\n"); + } + + ae.x = 0; + if (!ae.x) { + printf("Good\n"); + } else { + printf("Bad, ae.x is false\n"); + failures++; + } + + ae.y = 2222; + if (!ae.y) { + printf("Bad, ae.y is true (2222)\n"); + failures++; + } else { + printf("Good\n"); + } + + ae.y = 0; + if (!ae.y) { + printf("Good\n"); + } else { + printf("Bad, ae.y is false\n"); + failures++; + } + + ae.z = 111; + if (!ae.z) { + printf("Bad, ae.z is true (111)\n"); + failures++; + } else { + printf("Good\n"); + } + + ae.z = 0; + if (!ae.z) { + printf("Good\n"); + } else { + printf("Bad, ae.z is false\n"); + failures++; + } +} + +int main(void) +{ + test_if(); + test_if_not(); + printf("failures: %u\n", failures); + return failures; +}