diff --git a/src/cc65/codegen.c b/src/cc65/codegen.c index a81b2cf46..a0dbc498b 100644 --- a/src/cc65/codegen.c +++ b/src/cc65/codegen.c @@ -4455,7 +4455,7 @@ void g_testbitfield (unsigned Flags, unsigned BitOffs, unsigned BitWidth) -void g_extractbitfield (unsigned Flags, unsigned FullWidthFlags, +void g_extractbitfield (unsigned Flags, unsigned FullWidthFlags, int IsSigned, unsigned BitOffs, unsigned BitWidth) /* Extract bits from bit-field in ax. */ { @@ -4465,18 +4465,79 @@ void g_extractbitfield (unsigned Flags, unsigned FullWidthFlags, g_asr (Flags | CF_CONST, BitOffs); /* Since we have now shifted down, we could do char ops when the width fits in a char, but we - ** also need to clear the high byte since we've been using CF_FORCECHAR up to now. + ** also need to clear (or set) the high byte since we've been using CF_FORCECHAR up to now. */ + unsigned Mask = (1U << BitWidth) - 1; - /* 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, but we need to clear the high byte - ** for char. g_and emits no code if the mask is all ones. + /* To zero-extend, we will 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 will have already been shifted in, + ** but we need to clear the high byte for char. g_and emits no code if the mask is all ones. + ** This is here so the signed and unsigned branches can use it. */ + unsigned ZeroExtendMask = 0; /* Zero if we don't need to zero-extend. */ if (EndBit == CHAR_BITS) { /* We need to clear the high byte, since CF_FORCECHAR was set. */ - g_and (FullWidthFlags | CF_CONST, 0xFF); + ZeroExtendMask = 0xFF; } else if (EndBit != INT_BITS) { - g_and (FullWidthFlags | CF_CONST, (0x0001U << BitWidth) - 1U); + ZeroExtendMask = (1U << BitWidth) - 1; + } + + /* Handle signed bit-fields. */ + if (IsSigned) { + /* Push A, since the sign bit test will destroy it. */ + AddCodeLine ("pha"); + + /* Check sign bit */ + unsigned SignBitPos = BitWidth - 1U; + unsigned SignBitByte = SignBitPos / CHAR_BITS; + unsigned SignBitPosInByte = SignBitPos % CHAR_BITS; + unsigned SignBitMask = 1U << SignBitPosInByte; + + /* Move the correct byte to A. This can only be X for now, + ** but more cases will be needed to support long. + */ + switch (SignBitByte) { + case 0: + break; + case 1: + AddCodeLine ("txa"); + break; + default: + FAIL ("Invalid Byte for sign bit"); + } + + /* Test the sign bit */ + AddCodeLine ("and #$%02X", SignBitMask); + unsigned ZeroExtendLabel = GetLocalLabel (); + AddCodeLine ("beq %s", LocalLabelName (ZeroExtendLabel)); + + /* Pop A back and sign extend if required; operating on the full result need + ** to sign-extend into high byte, too. + */ + AddCodeLine ("pla"); + g_or (FullWidthFlags | CF_CONST, ~Mask); + + /* Apparently, there is no unconditional branch BRA, so use JMP. */ + unsigned DoneLabel = GetLocalLabel (); + AddCodeLine ("jmp %s", LocalLabelName (DoneLabel)); + + /* Pop A back, then zero-extend; we need to duplicate the PLA rather than move it before + ** the branch to share with the other label because PLA sets the condition codes. + */ + g_defcodelabel (ZeroExtendLabel); + AddCodeLine ("pla"); + + /* Zero the upper bits, the same as the unsigned path. */ + if (ZeroExtendMask != 0) { + g_and (FullWidthFlags | CF_CONST, ZeroExtendMask); + } + + g_defcodelabel (DoneLabel); + } else { + /* Unsigned bit-field, only needs zero-extension. */ + if (ZeroExtendMask != 0) { + g_and (FullWidthFlags | CF_CONST, ZeroExtendMask); + } } } diff --git a/src/cc65/codegen.h b/src/cc65/codegen.h index 6581fc54e..ec4756f2d 100644 --- a/src/cc65/codegen.h +++ b/src/cc65/codegen.h @@ -478,7 +478,7 @@ void g_initstatic (unsigned InitLabel, unsigned VarLabel, unsigned Size); void g_testbitfield (unsigned Flags, unsigned BitOffs, unsigned BitWidth); /* Test bit-field in ax. */ -void g_extractbitfield (unsigned Flags, unsigned FullWidthFlags, +void g_extractbitfield (unsigned Flags, unsigned FullWidthFlags, int IsSigned, unsigned BitOffs, unsigned BitWidth); /* Extract bits from bit-field in ax. */ diff --git a/src/cc65/declare.c b/src/cc65/declare.c index 44d4e4142..ccd4e9004 100644 --- a/src/cc65/declare.c +++ b/src/cc65/declare.c @@ -41,6 +41,7 @@ /* common */ #include "addrsize.h" #include "mmodel.h" +#include "shift.h" #include "xmalloc.h" /* cc65 */ @@ -87,7 +88,8 @@ struct StructInitData { -static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers); +static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers, + int* SignednessSpecified); /* Parse a type specifier */ static unsigned ParseInitInternal (Type* T, int* Braces, int AllowFlexibleMembers); @@ -252,12 +254,15 @@ static void OptionalInt (void) -static void OptionalSigned (void) +static void OptionalSigned (int* SignednessSpecified) /* Eat an optional "signed" token */ { if (CurTok.Tok == TOK_SIGNED) { /* Skip it */ NextToken (); + if (SignednessSpecified != NULL) { + *SignednessSpecified = 1; + } } } @@ -728,6 +733,9 @@ static int ParseFieldWidth (Declaration* Decl) return -1; } + /* TODO: This can be relaxed to be any integral type, but + ** ParseStructInit currently only supports up to int. + */ if (SizeOf (Decl->Type) != SizeOf (type_uint)) { /* Only int sized types may be used for bit-fields for now */ Error ("cc65 currently only supports unsigned int bit-fields"); @@ -774,7 +782,8 @@ static unsigned PadWithBitField (unsigned StructSize, unsigned BitOffs) /* Add an anonymous bit-field that aligns to the next ** byte. */ - AddBitField (Ident, StructSize, BitOffs, PaddingBits); + AddBitField (Ident, type_uchar, StructSize, BitOffs, PaddingBits, + /*SignednessSpecified=*/1); return PaddingBits; } @@ -866,8 +875,9 @@ static SymEntry* ParseUnionDecl (const char* Name) /* Get the type of the entry */ DeclSpec Spec; + int SignednessSpecified = 0; InitDeclSpec (&Spec); - ParseTypeSpec (&Spec, -1, T_QUAL_NONE); + ParseTypeSpec (&Spec, -1, T_QUAL_NONE, &SignednessSpecified); /* Read fields with this type */ while (1) { @@ -909,7 +919,11 @@ static SymEntry* ParseUnionDecl (const char* Name) /* Add a field entry to the table. */ if (FieldWidth > 0) { - AddBitField (Decl.Ident, 0, 0, FieldWidth); + /* For a union, allocate space for the type specified by the + ** bit-field. + */ + AddBitField (Decl.Ident, Decl.Type, 0, 0, FieldWidth, + SignednessSpecified); } else { if (IsAnonName (Decl.Ident)) { Entry = AddLocalSym (Decl.Ident, Decl.Type, SC_STRUCTFIELD, 0); @@ -997,8 +1011,9 @@ static SymEntry* ParseStructDecl (const char* Name) continue; } + int SignednessSpecified = 0; InitDeclSpec (&Spec); - ParseTypeSpec (&Spec, -1, T_QUAL_NONE); + ParseTypeSpec (&Spec, -1, T_QUAL_NONE, &SignednessSpecified); /* Read fields with this type */ while (1) { @@ -1020,12 +1035,13 @@ static SymEntry* ParseStructDecl (const char* Name) FieldWidth = ParseFieldWidth (&Decl); /* If this is not a bit field, or the bit field is too large for - ** the remainder of the current member, or we have a bit field + ** the remainder of the allocated unit, or we have a bit field ** with width zero, align the struct to the next member by adding ** a member with an anonymous name. */ if (BitOffs > 0) { - if (FieldWidth <= 0 || (BitOffs + FieldWidth) > INT_BITS) { + if (FieldWidth <= 0 || + (BitOffs + FieldWidth) > CHAR_BITS * SizeOf (Decl.Type)) { /* Add an anonymous bit-field that aligns to the next ** byte. */ @@ -1087,9 +1103,10 @@ static SymEntry* ParseStructDecl (const char* Name) ** bit-field as a char type in expressions. */ CHECK (BitOffs < CHAR_BITS); - AddBitField (Decl.Ident, StructSize, BitOffs, FieldWidth); + AddBitField (Decl.Ident, Decl.Type, StructSize, BitOffs, + FieldWidth, SignednessSpecified); BitOffs += FieldWidth; - CHECK (BitOffs <= INT_BITS); + CHECK (BitOffs <= CHAR_BITS * SizeOf (Decl.Type)); /* Add any full bytes to the struct size. */ StructSize += BitOffs / CHAR_BITS; BitOffs %= CHAR_BITS; @@ -1145,12 +1162,20 @@ NextMember: if (CurTok.Tok != TOK_COMMA) { -static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers) -/* Parse a type specifier */ +static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers, + int* SignednessSpecified) +/* Parse a type specifier. Store whether one of "signed" or "unsigned" was +** specified, so bit-fields of unspecified signedness can be treated as +** unsigned; without special handling, it would be treated as signed. +*/ { ident Ident; SymEntry* Entry; + if (SignednessSpecified != NULL) { + *SignednessSpecified = 0; + } + /* Assume we have an explicit type */ D->Flags &= ~DS_DEF_TYPE; @@ -1176,12 +1201,15 @@ static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers) case TOK_LONG: NextToken (); if (CurTok.Tok == TOK_UNSIGNED) { + if (SignednessSpecified != NULL) { + *SignednessSpecified = 1; + } NextToken (); OptionalInt (); D->Type[0].C = T_ULONG; D->Type[1].C = T_END; } else { - OptionalSigned (); + OptionalSigned (SignednessSpecified); OptionalInt (); D->Type[0].C = T_LONG; D->Type[1].C = T_END; @@ -1191,12 +1219,15 @@ static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers) case TOK_SHORT: NextToken (); if (CurTok.Tok == TOK_UNSIGNED) { + if (SignednessSpecified != NULL) { + *SignednessSpecified = 1; + } NextToken (); OptionalInt (); D->Type[0].C = T_USHORT; D->Type[1].C = T_END; } else { - OptionalSigned (); + OptionalSigned (SignednessSpecified); OptionalInt (); D->Type[0].C = T_SHORT; D->Type[1].C = T_END; @@ -1210,6 +1241,9 @@ static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers) break; case TOK_SIGNED: + if (SignednessSpecified != NULL) { + *SignednessSpecified = 1; + } NextToken (); switch (CurTok.Tok) { @@ -1245,6 +1279,9 @@ static void ParseTypeSpec (DeclSpec* D, long Default, TypeCode Qualifiers) break; case TOK_UNSIGNED: + if (SignednessSpecified != NULL) { + *SignednessSpecified = 1; + } NextToken (); switch (CurTok.Tok) { @@ -1835,7 +1872,7 @@ Type* ParseType (Type* T) /* Get a type without a default */ InitDeclSpec (&Spec); - ParseTypeSpec (&Spec, -1, T_QUAL_NONE); + ParseTypeSpec (&Spec, -1, T_QUAL_NONE, NULL); /* Parse additional declarators */ ParseDecl (&Spec, &Decl, DM_NO_IDENT); @@ -1967,7 +2004,7 @@ void ParseDeclSpec (DeclSpec* D, unsigned DefStorage, long DefType) ParseStorageClass (D, DefStorage); /* Parse the type specifiers passing any initial type qualifiers */ - ParseTypeSpec (D, DefType, Qualifiers); + ParseTypeSpec (D, DefType, Qualifiers, NULL); } @@ -2362,6 +2399,7 @@ static unsigned ParseStructInit (Type* T, int* Braces, int AllowFlexibleMembers) ** into 3 bytes. */ SI.ValBits += Entry->V.B.BitWidth; + /* TODO: Generalize this so any type can be used. */ CHECK (SI.ValBits <= CHAR_BITS + INT_BITS - 2); while (SI.ValBits >= CHAR_BITS) { OutputBitFieldData (&SI); @@ -2393,16 +2431,34 @@ static unsigned ParseStructInit (Type* T, int* Braces, int AllowFlexibleMembers) SI.Offs * CHAR_BITS + SI.ValBits); /* Read the data, check for a constant integer, do a range check */ - ParseScalarInitInternal (type_uint, &ED); + ParseScalarInitInternal (Entry->Type, &ED); if (!ED_IsConstAbsInt (&ED)) { Error ("Constant initializer expected"); ED_MakeConstAbsInt (&ED, 1); } - if (ED.IVal > (long) Mask) { - Warning ("Truncating value in bit-field initializer"); - ED.IVal &= (long) Mask; + + /* Truncate the initializer value to the width of the bit-field and check if we lost + ** any useful bits. + */ + Val = (unsigned) ED.IVal & Mask; + if (IsSignUnsigned (Entry->Type)) { + if (ED.IVal < 0 || (unsigned long) ED.IVal != Val) { + Warning ("Implicit truncation from '%s' to '%s : %u' in bit-field initializer" + " changes value from %ld to %u", + GetFullTypeName (ED.Type), GetFullTypeName (Entry->Type), + Entry->V.B.BitWidth, ED.IVal, Val); + } + } else { + /* Sign extend back to full width of host long. */ + unsigned ShiftBits = sizeof (long) * CHAR_BIT - Entry->V.B.BitWidth; + long RestoredVal = asr_l(asl_l (Val, ShiftBits), ShiftBits); + if (ED.IVal != RestoredVal) { + Warning ("Implicit truncation from '%s' to '%s : %u' in bit-field initializer " + "changes value from %ld to %d", + GetFullTypeName (ED.Type), GetFullTypeName (Entry->Type), + Entry->V.B.BitWidth, ED.IVal, Val); + } } - Val = (unsigned) ED.IVal; /* Add the value to the currently stored bit-field value */ Shift = (Entry->V.B.Offs - SI.Offs) * CHAR_BITS + Entry->V.B.BitOffs; @@ -2417,6 +2473,7 @@ static unsigned ParseStructInit (Type* T, int* Braces, int AllowFlexibleMembers) ** aligned, so will have padding before it. */ CHECK (SI.ValBits <= CHAR_BIT * sizeof(SI.BitVal)); + /* TODO: Generalize this so any type can be used. */ CHECK (SI.ValBits <= CHAR_BITS + INT_BITS - 2); while (SI.ValBits >= CHAR_BITS) { OutputBitFieldData (&SI); @@ -2425,7 +2482,8 @@ static unsigned ParseStructInit (Type* T, int* Braces, int AllowFlexibleMembers) } else { /* Standard member. We should never have stuff from a - ** bit-field left + ** bit-field left because an anonymous member was added + ** for padding by ParseStructDecl. */ CHECK (SI.ValBits == 0); diff --git a/src/cc65/loadexpr.c b/src/cc65/loadexpr.c index 452d9a9a3..f3a1a6add 100644 --- a/src/cc65/loadexpr.c +++ b/src/cc65/loadexpr.c @@ -123,7 +123,9 @@ void LoadExpr (unsigned Flags, struct ExprDesc* Expr) ** supported. */ Flags |= (EndBit <= CHAR_BITS) ? CF_CHAR : CF_INT; - Flags |= CF_UNSIGNED; + if (IsSignUnsigned (Expr->Type)) { + Flags |= CF_UNSIGNED; + } /* Flags we need operate on the whole bit-field, without CF_FORCECHAR. */ BitFieldFullWidthFlags = Flags; @@ -133,7 +135,11 @@ void LoadExpr (unsigned Flags, struct ExprDesc* Expr) ** type is not CF_CHAR. */ if (AdjustBitField) { - Flags |= CF_FORCECHAR; + /* If adjusting, then we're sign extending manually, so do everything unsigned + ** to make shifts faster. + */ + Flags |= CF_UNSIGNED | CF_FORCECHAR; + BitFieldFullWidthFlags |= CF_UNSIGNED; } } else if ((Flags & CF_TYPEMASK) == 0) { Flags |= TypeOf (Expr->Type); @@ -231,7 +237,8 @@ void LoadExpr (unsigned Flags, struct ExprDesc* Expr) if (ED_NeedsTest (Expr)) { g_testbitfield (Flags, Expr->BitOffs, Expr->BitWidth); } else { - g_extractbitfield (Flags, BitFieldFullWidthFlags, Expr->BitOffs, Expr->BitWidth); + g_extractbitfield (Flags, BitFieldFullWidthFlags, IsSignSigned (Expr->Type), + Expr->BitOffs, Expr->BitWidth); } } diff --git a/src/cc65/symtab.c b/src/cc65/symtab.c index 6e0654576..7e6a9f5bb 100644 --- a/src/cc65/symtab.c +++ b/src/cc65/symtab.c @@ -819,7 +819,8 @@ SymEntry* AddStructSym (const char* Name, unsigned Flags, unsigned Size, SymTabl -SymEntry* AddBitField (const char* Name, unsigned Offs, unsigned BitOffs, unsigned BitWidth) +SymEntry* AddBitField (const char* Name, const Type* T, unsigned Offs, + unsigned BitOffs, unsigned BitWidth, int SignednessSpecified) /* Add a bit field to the local symbol table and return the symbol entry */ { /* Do we have an entry with this name already? */ @@ -834,12 +835,21 @@ SymEntry* AddBitField (const char* Name, unsigned Offs, unsigned BitOffs, unsign /* Create a new entry */ Entry = NewSymEntry (Name, SC_BITFIELD); - /* Set the symbol attributes. Bit-fields are always of type unsigned */ - Entry->Type = type_uint; + /* Set the symbol attributes. Bit-fields are always integral types. */ + Entry->Type = TypeDup (T); Entry->V.B.Offs = Offs; Entry->V.B.BitOffs = BitOffs; Entry->V.B.BitWidth = BitWidth; + if (!SignednessSpecified) { + /* int is treated as signed int everywhere except bit-fields; switch it to unsigned, + ** since this is allowed for bit-fields and avoids sign-extension, so is much faster. + */ + CHECK ((Entry->Type->C & T_MASK_SIGN) == T_SIGN_SIGNED); + Entry->Type->C &= ~T_MASK_SIGN; + Entry->Type->C |= T_SIGN_UNSIGNED; + } + /* Add the entry to the symbol table */ AddSymEntry (SymTab, Entry); diff --git a/src/cc65/symtab.h b/src/cc65/symtab.h index f04517fed..750e41b54 100644 --- a/src/cc65/symtab.h +++ b/src/cc65/symtab.h @@ -155,7 +155,8 @@ SymEntry* AddEnumSym (const char* Name, unsigned Flags, const Type* Type, SymTab SymEntry* AddStructSym (const char* Name, unsigned Flags, unsigned Size, SymTable* Tab); /* Add a struct/union entry and return it */ -SymEntry* AddBitField (const char* Name, unsigned Offs, unsigned BitOffs, unsigned BitWidth); +SymEntry* AddBitField (const char* Name, const Type* Type, unsigned Offs, + unsigned BitOffs, unsigned BitWidth, int SignednessSpecified); /* Add a bit field to the local symbol table and return the symbol entry */ SymEntry* AddConstSym (const char* Name, const Type* T, unsigned Flags, long Val); diff --git a/test/todo/bug1095.c b/test/val/bug1095.c similarity index 74% rename from test/todo/bug1095.c rename to test/val/bug1095.c index 0803c2d1c..8c184cd1d 100644 --- a/test/todo/bug1095.c +++ b/test/val/bug1095.c @@ -31,7 +31,10 @@ static struct signed_ints { signed int b : 3; signed int c : 3; signed int d : 10; -} si = {-4, -1, 3, -500}; + signed int : 0; + signed int e : 8; + signed int f : 16; +} si = {-4, -1, 3, -500, -100, -5000}; static void test_signed_bitfield(void) { @@ -53,7 +56,7 @@ static void test_signed_bitfield(void) failures++; } - if (si.b <= 0) { + if (si.c <= 0) { printf("Got si.c = %d, expected positive.\n", si.c); failures++; } @@ -71,10 +74,30 @@ static void test_signed_bitfield(void) failures++; } + if (si.e >= 0) { + printf("Got si.e = %d, expected negative.\n", si.e); + failures++; + } + if (si.e != -100) { + printf("Got si.e = %d, expected -100.\n", si.e); + failures++; + } + + if (si.f >= 0) { + printf("Got si.f = %d, expected negative.\n", si.f); + failures++; + } + if (si.f != -5000) { + printf("Got si.f = %d, expected -5000.\n", si.f); + failures++; + } + si.a = -3; si.b = 1; si.c = -2; si.d = 500; + si.e = 100; + si.f = 5000; if (si.a >= 0) { printf("Got si.a = %d, expected negative.\n", si.a); @@ -94,7 +117,7 @@ static void test_signed_bitfield(void) failures++; } - if (si.b >= 0) { + if (si.c >= 0) { printf("Got si.c = %d, expected negative.\n", si.c); failures++; } @@ -111,6 +134,24 @@ static void test_signed_bitfield(void) printf("Got si.d = %d, expected 500.\n", si.d); failures++; } + + if (si.e <= 0) { + printf("Got si.e = %d, expected positive.\n", si.e); + failures++; + } + if (si.e != 100) { + printf("Got si.e = %d, expected 100.\n", si.e); + failures++; + } + + if (si.f <= 0) { + printf("Got si.f = %d, expected positive.\n", si.f); + failures++; + } + if (si.f != 5000) { + printf("Got si.f = %d, expected 5000.\n", si.f); + failures++; + } } int main(void)