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

[Stepan Dyatkovskiy]

OK. Please look at patch in attachment.
I'm not sure that it is better than previous patch. Probably the first one looks like a workaround, but it changes setSubclassData only. New patch changes set/getSubclassData set/getTypeID, and all methods that uses ID.

-Stepan.

02.01.2012, 15:04, "Duncan Sands" <baldrick at free.fr>:
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 11652-2.0.patch
Type: application/octet-stream
Size: 10162 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120102/e7f1d18c/attachment.obj>