[llvm-commits] [LLVM, PR11652 PATCH]: Fixed Bug 11652 - assertion failures when Type.cpp is compiled with -Os

[Duncan Sands]

Hi Stepan,

> ID is used very extensively in Type.h. We need to fix a lots, so we need to fix all methods like:
> bool isIntegerTy() const { return ID == IntegerTyID; }

you could turn ID into a private method that extracts the id part of the field.
Then you just need to turn ID into ID() in places such as isIntegerTy.  Likewise
for SubclassData.

> But in the same time we can apply some working decision until gcc bug will fixed.
> May be add some dummy field?
>    TypeID   ID : 8;
>    unsigned SubclassData : 24;
>    unsigned KungFuPanda;        // Will protect NumContainedTys from overwriting.
>    unsigned NumContainedTys; // Will OK.

Even if the gcc bug is fixed, people will be using  older compilers with the bug
for years to come.  So this field would be around essentially forever.  Given
that, I don't think this is a good solution.  If you are prepared to make the
class bigger, you might as well not have the fields be bitfields at all (and
change the order so that things are well packed).

Ciao, Duncan.

>
> -Stepan.
>
> 02.01.2012, 14:38, "Duncan Sands"<baldrick at free.fr>:
>> Hi Stepan,
>>
>>>   I tried it doesn't helps. Now it seems that ID is overwritten. 4807 unexpected failures.
>>
>> OK, thanks for the info.  How about doing the bit fiddling yourself instead?
>> I.e. rather than trying to fool the optimizers, don't use bitfields: declare
>> an unsigned field IDAndSubclassData and store and load values from it using
>> explicit shifts etc.  This would then completely avoid all problems coming
>> from misoptimization of bitfields (which has happened a lot historically),
>> and would be less fragile than trying to fool the optimizers via some magic
>> incantation.
>>
>> Ciao, Duncan.
>>
>>>   -Stepan.
>>>
>>>   02.01.2012, 14:02, "Duncan Sands"<baldrick at free.fr>:
>>>>   Hi Stepan,
>>>>>     The problem is in Type.h. The fields in Type class are declared in next order:
>>>>>     TypeID ID : 8;
>>>>>     unsigned SubclassData : 24;
>>>>>     unsigned NumContainedTys;
>>>>   does the problem still occur if you flip the order of ID and SubclassData?
>>>>   I.e.
>>>>       unsigned SubclassData : 24;
>>>>       TypeID ID : 8;
>>>>       unsigned NumContainedTys;
>>>>   ?
>>>>   Ciao, Duncan.
>>>>>     Attempt to set new SubclassData value rewrites lowest byte in NumContainedTys
>>>>>     when -Os is set. GCC bug? Anyway setting SubclassData with two workaround
>>>>>     strings fixes the problem:
>>>>>
>>>>>     void setSubclassData(unsigned val) {
>>>>>     unsigned tmp = NumContainedTys; // Workaround for GCC -Os
>>>>>     SubclassData = val;
>>>>>     NumContainedTys = tmp; // Workaround for GCC -Os
>>>>>     // Ensure we don't have any accidental truncation.
>>>>>     assert(SubclassData == val&&    "Subclass data too large for field");
>>>>>     }
>>>>>
>>>>>     Probably there is another ways to protect NumContainedTys from overwritting?
>>>>>
>>>>>     Please find the patch in attachment for review.
>>>>>
>>>>>     -Stepan.
>>>>>
>>>>>     _______________________________________________
>>>>>     llvm-commits mailing list
>>>>>     llvm-commits at cs.uiuc.edu
>>>>>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>   _______________________________________________
>>>>   llvm-commits mailing list
>>>>   llvm-commits at cs.uiuc.edu
>>>>   http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits