[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan created this revision.
ebevhan added reviewers: jasonliu, efriedma.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
ebevhan requested review of this revision.

In D79719 , LayoutField was refactored to 
fetch the size of field
types in bits and then convert to chars, rather than fetching
them in chars directly. This is not ideal, since it makes the
calculations char size dependent, and breaks for sizes that
are not a multiple of the char size.

This patch changes it to use getTypeInfoInChars instead of
getTypeInfo.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85191

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp


Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1838,14 +1838,13 @@
   CharUnits EffectiveFieldSize;
 
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;
 // Flexible array members don't have any size, but they have to be
 // aligned appropriately for their element type.
 EffectiveFieldSize = FieldSize =
-IsIncompleteArrayType ? CharUnits::Zero()
-  : Context.toCharUnitsFromBits(TI.Width);
-AlignIsRequired = TI.AlignIsRequired;
+IsIncompleteArrayType ? CharUnits::Zero() : TI.first;
+AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired;
   };
 
   if (D->getType()->isIncompleteArrayType()) {


Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1838,14 +1838,13 @@
   CharUnits EffectiveFieldSize;
 
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;
 // Flexible array members don't have any size, but they have to be
 // aligned appropriately for their element type.
 EffectiveFieldSize = FieldSize =
-IsIncompleteArrayType ? CharUnits::Zero()
-  : Context.toCharUnitsFromBits(TI.Width);
-AlignIsRequired = TI.AlignIsRequired;
+IsIncompleteArrayType ? CharUnits::Zero() : TI.first;
+AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired;
   };
 
   if (D->getType()->isIncompleteArrayType()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1841
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;

In most cases, `getTypeInfoInChars` invokes `getTypeInfo` underneath. So to 
make people be careful about this, I would suggest to leave a comment 
explaining/claiming we have to call `getTypeInfoInChars` here. And also maybe 
adding a testcase to guard the scenario you were talking about would be helpful 
to prevent someone to use `getTypeInfo` here in the future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1841
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;

Xiangling_L wrote:
> In most cases, `getTypeInfoInChars` invokes `getTypeInfo` underneath. So to 
> make people be careful about this, I would suggest to leave a comment 
> explaining/claiming we have to call `getTypeInfoInChars` here. And also maybe 
> adding a testcase to guard the scenario you were talking about would be 
> helpful to prevent someone to use `getTypeInfo` here in the future.
I can do that.

I honestly don't think it would be a bad idea to add an assertion to 
toCharUnitsFromBits that checks for non-bytesize-multiple amounts. I wonder how 
much would fail if I did that, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

> This is not ideal, since it makes the

calculations char size dependent, and breaks for sizes that
are not a multiple of the char size.

How can we have a non-bitfield member whose size is not a multiple of the size 
of a char?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D85191#2193645 , @rsmith wrote:

>> This is not ideal, since it makes the calculations char size dependent, and 
>> breaks for sizes that are not a multiple of the char size.
>
> How can we have a non-bitfield member whose size is not a multiple of the 
> size of a char?

Downstream, we have fixed-point types that are 24 bits large, but where the 
char size is 16. The type then takes up 2 chars, where 8 of the bits are 
padding. The only way in Clang to express that the width of the bit 
representation of a type should be smaller than the number of chars it takes up 
in memory -- and consequently, produce an `i24` in IR -- is to return a 
non-charsize multiple from getTypeInfo.

We did it this way because it was possible. If the intent is for getTypeInfo to 
always return sizes that are multiples of the char size, then the design should 
be inverted and getTypeInfo should simply be calling getTypeInfoInChars and 
multiply that result by the char size. But that isn't how it works.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1841
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;

ebevhan wrote:
> Xiangling_L wrote:
> > In most cases, `getTypeInfoInChars` invokes `getTypeInfo` underneath. So to 
> > make people be careful about this, I would suggest to leave a comment 
> > explaining/claiming we have to call `getTypeInfoInChars` here. And also 
> > maybe adding a testcase to guard the scenario you were talking about would 
> > be helpful to prevent someone to use `getTypeInfo` here in the future.
> I can do that.
> 
> I honestly don't think it would be a bad idea to add an assertion to 
> toCharUnitsFromBits that checks for non-bytesize-multiple amounts. I wonder 
> how much would fail if I did that, though.
Oh, I guess I only really replied to the first part about adding a comment 
here... I'm not sure I can make a test case for this, since I don't think 
there's anything that triggers this upstream.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D85191#2195923 , @ebevhan wrote:

> In D85191#2193645 , @rsmith wrote:
>
>>> This is not ideal, since it makes the calculations char size dependent, and 
>>> breaks for sizes that are not a multiple of the char size.
>>
>> How can we have a non-bitfield member whose size is not a multiple of the 
>> size of a char?
>
> Downstream, we have fixed-point types that are 24 bits large, but where the 
> char size is 16. The type then takes up 2 chars, where 8 of the bits are 
> padding. The only way in Clang to express that the width of the bit 
> representation of a type should be smaller than the number of chars it takes 
> up in memory -- and consequently, produce an `i24` in IR -- is to return a 
> non-charsize multiple from getTypeInfo.

This violates the C and C++ language rules, which require the size of every 
type to be a multiple of the size of char. A type with 24 value bits and 8 
padding bits should report a type size of 32 bits, just like `bool` reports a 
size of `CHAR_BIT` bits despite having only 1 value bit, and x86_64 `long 
double` reports a type size of 128 bits despite having only 80 value bits.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

In D85191#2196863 , @rsmith wrote:

> In D85191#2195923 , @ebevhan wrote:
>
>> In D85191#2193645 , @rsmith wrote:
>>
 This is not ideal, since it makes the calculations char size dependent, 
 and breaks for sizes that are not a multiple of the char size.
>>>
>>> How can we have a non-bitfield member whose size is not a multiple of the 
>>> size of a char?
>>
>> Downstream, we have fixed-point types that are 24 bits large, but where the 
>> char size is 16. The type then takes up 2 chars, where 8 of the bits are 
>> padding. The only way in Clang to express that the width of the bit 
>> representation of a type should be smaller than the number of chars it takes 
>> up in memory -- and consequently, produce an `i24` in IR -- is to return a 
>> non-charsize multiple from getTypeInfo.
>
> This violates the C and C++ language rules, which require the size of every 
> type to be a multiple of the size of char. A type with 24 value bits and 8 
> padding bits should report a type size of 32 bits, just like `bool` reports a 
> size of `CHAR_BIT` bits despite having only 1 value bit, and x86_64 `long 
> double` reports a type size of 128 bits despite having only 80 value bits.

I don't see that it breaks the language rules. The sizeof result for the 24 bit 
type should be 2 in the target described by @ebevhan  (two 16-bit bytes). But I 
imagine that without this patch it is reported as 24/16=1, right?

So isn't the problem that toCharUnitsFromBits is rounding down when given a 
bitsize that isn't a multiple of CHAR_BIT? Would it perhaps make sense to let 
it round up instead?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> If the intent is for getTypeInfo to always return sizes that are multiples of 
> the char size, then the design should be inverted and getTypeInfo should 
> simply be calling getTypeInfoInChars and multiply that result by the char 
> size. But that isn't how it works.

The reason it doesn't work this way is just that someone made the wrong choice 
a decade ago, and nobody has spent the time to rewrite it since.  Patch welcome.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D85191#2197550 , @bjope wrote:

> In D85191#2196863 , @rsmith wrote:
>
>> In D85191#2195923 , @ebevhan wrote:
>>
>>> In D85191#2193645 , @rsmith wrote:
>>>
> This is not ideal, since it makes the calculations char size dependent, 
> and breaks for sizes that are not a multiple of the char size.

 How can we have a non-bitfield member whose size is not a multiple of the 
 size of a char?
>>>
>>> Downstream, we have fixed-point types that are 24 bits large, but where the 
>>> char size is 16. The type then takes up 2 chars, where 8 of the bits are 
>>> padding. The only way in Clang to express that the width of the bit 
>>> representation of a type should be smaller than the number of chars it 
>>> takes up in memory -- and consequently, produce an `i24` in IR -- is to 
>>> return a non-charsize multiple from getTypeInfo.
>>
>> This violates the C and C++ language rules, which require the size of every 
>> type to be a multiple of the size of char. A type with 24 value bits and 8 
>> padding bits should report a type size of 32 bits, just like `bool` reports 
>> a size of `CHAR_BIT` bits despite having only 1 value bit, and x86_64 `long 
>> double` reports a type size of 128 bits despite having only 80 value bits.
>
> I don't see that it breaks the language rules. The sizeof result for the 24 
> bit type should be 2 in the target described by @ebevhan  (two 16-bit bytes). 
> But I imagine that without this patch it is reported as 24/16=1, right?

Yes, this is what's happening. The sizeof should be reported as 2 (32 bits), 
but isn't, because toCharUnitsFromBits always rounds down.

> So isn't the problem that toCharUnitsFromBits is rounding down when given a 
> bitsize that isn't a multiple of CHAR_BIT? Would it perhaps make sense to let 
> it round up instead?

The issue with toCharUnitsFromBits is that it's an inherently dangerous API. 
There could be cases where you want to round down, and cases where you want to 
round up. The function cannot know.

It could be better if toCharUnitsFromBits took an extra parameter that 
explicitly specifies the rounding, and if that parameter is set to a default 
(for unspecified rounding) and the amount passed is not a multiple of the char 
size, it asserts. This would make a lot of tests fail until all of the uses are 
corrected, though.

In D85191#2196863 , @rsmith wrote:

> In D85191#2195923 , @ebevhan wrote:
>
>> In D85191#2193645 , @rsmith wrote:
>>
 This is not ideal, since it makes the calculations char size dependent, 
 and breaks for sizes that are not a multiple of the char size.
>>>
>>> How can we have a non-bitfield member whose size is not a multiple of the 
>>> size of a char?
>>
>> Downstream, we have fixed-point types that are 24 bits large, but where the 
>> char size is 16. The type then takes up 2 chars, where 8 of the bits are 
>> padding. The only way in Clang to express that the width of the bit 
>> representation of a type should be smaller than the number of chars it takes 
>> up in memory -- and consequently, produce an `i24` in IR -- is to return a 
>> non-charsize multiple from getTypeInfo.
>
> This violates the C and C++ language rules, which require the size of every 
> type to be a multiple of the size of char. A type with 24 value bits and 8 
> padding bits should report a type size of 32 bits, just like `bool` reports a 
> size of `CHAR_BIT` bits despite having only 1 value bit, and x86_64 `long 
> double` reports a type size of 128 bits despite having only 80 value bits.

But this is the crux of the matter; if you aren't allowed to return 
non-char-sizes from getTypeInfo, then there's no way to specify via 
TargetInfo/getTypeInfo that the number of value bits of a type is less than the 
size in chars. That is, that a type is padded. And as your examples show, C 
does not disallow that.

In D85191#2197663 , @efriedma wrote:

>> If the intent is for getTypeInfo to always return sizes that are multiples 
>> of the char size, then the design should be inverted and getTypeInfo should 
>> simply be calling getTypeInfoInChars and multiply that result by the char 
>> size. But that isn't how it works.
>
> The reason it doesn't work this way is just that someone made the wrong 
> choice a decade ago, and nobody has spent the time to rewrite it since.  
> Patch welcome.

This does sound like a good thing to do, but it would be problematic downstream 
since it would completely prohibit the design that we're trying to use.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I don't know whether the name of your downstream target is a secret. Wouldn't 
it help you to add a fake 16bit per char target and add units tests to prevent 
regressions?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D85191#2199574 , @ebevhan wrote:

> In D85191#2197663 , @efriedma wrote:
>
>>> If the intent is for getTypeInfo to always return sizes that are multiples 
>>> of the char size, then the design should be inverted and getTypeInfo should 
>>> simply be calling getTypeInfoInChars and multiply that result by the char 
>>> size. But that isn't how it works.
>>
>> The reason it doesn't work this way is just that someone made the wrong 
>> choice a decade ago, and nobody has spent the time to rewrite it since.  
>> Patch welcome.
>
> This does sound like a good thing to do, but it would be problematic 
> downstream since it would completely prohibit the design that we're trying to 
> use.

That would be a good thing, as the design you're trying to use is not one that 
we intend to support. The size returned by `getTypeInfo` is intended to include 
padding.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

It doesn't feel like this patch got a very positive reception, but I'd still 
like to try a bit more to get it in.

Even though it's difficult to test this particular change upstream, would it 
still be acceptable to take the patch since it reverts the behavior to what it 
was previously? If there are worries that things may break in the future due to 
other changes, we do catch these things in our downstream testing and are 
fairly diligent about reporting back about them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm still concerned your approach to the computation of getTypeSize() is a 
ticking time bomb, but I'll take the cleanup even if the underlying motivation 
doesn't really make sense.




Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1847
+IsIncompleteArrayType ? CharUnits::Zero() : TI.first;
+AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired;
   };

Can we fix getTypeInfoInChars so it returns all the necessary info, so we don't 
look up the typeinfo twice?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1847
+IsIncompleteArrayType ? CharUnits::Zero() : TI.first;
+AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired;
   };

efriedma wrote:
> Can we fix getTypeInfoInChars so it returns all the necessary info, so we 
> don't look up the typeinfo twice?
That feels like a hefty change since it would require changing every caller of 
getTypeInfoInChars. Do you want me to do that in this patch or in a separate 
one?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1847
+IsIncompleteArrayType ? CharUnits::Zero() : TI.first;
+AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired;
   };

ebevhan wrote:
> efriedma wrote:
> > Can we fix getTypeInfoInChars so it returns all the necessary info, so we 
> > don't look up the typeinfo twice?
> That feels like a hefty change since it would require changing every caller 
> of getTypeInfoInChars. Do you want me to do that in this patch or in a 
> separate one?
Separate patch makes sense.

If you want to do it after this one, that's fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e7ec4842c1a: [AST] Get field size in chars rather than bits 
in RecordLayoutBuilder. (authored by ebevhan).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp


Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1838,14 +1838,13 @@
   CharUnits EffectiveFieldSize;
 
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;
 // Flexible array members don't have any size, but they have to be
 // aligned appropriately for their element type.
 EffectiveFieldSize = FieldSize =
-IsIncompleteArrayType ? CharUnits::Zero()
-  : Context.toCharUnitsFromBits(TI.Width);
-AlignIsRequired = TI.AlignIsRequired;
+IsIncompleteArrayType ? CharUnits::Zero() : TI.first;
+AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired;
   };
 
   if (D->getType()->isIncompleteArrayType()) {


Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1838,14 +1838,13 @@
   CharUnits EffectiveFieldSize;
 
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;
 // Flexible array members don't have any size, but they have to be
 // aligned appropriately for their element type.
 EffectiveFieldSize = FieldSize =
-IsIncompleteArrayType ? CharUnits::Zero()
-  : Context.toCharUnitsFromBits(TI.Width);
-AlignIsRequired = TI.AlignIsRequired;
+IsIncompleteArrayType ? CharUnits::Zero() : TI.first;
+AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired;
   };
 
   if (D->getType()->isIncompleteArrayType()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits