[CIR][Codegen] Initial support for packed structures #473

gitoleg · 2024-02-20T12:36:48Z

This PR adds a support for packed structures.

Basically, now both pragma pack(...) and __attribute__((aligned(...))) should work.
The only problem is that getAlignment is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

github-actions · 2024-02-20T12:39:09Z

✅ With the latest revision this PR passed the C/C++ code formatter.

gitoleg · 2024-02-21T10:56:38Z

@bcardosolopes
tests fail :( due to the struct type I created earlier. The problem is described here in #476. Briefly, the struct is considered as the packed one, which is not true. Time to think how to handle it.

This PR intends to fix some problems with packed structures support, so the #473 will work. ### Problems Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. ### Why Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. ### Implementation details 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

gitoleg · 2024-03-14T09:12:41Z

@bcardosolopes Looks like we this one is ready for review (after #487 merged)

bcardosolopes

Awesome, LGTM. I'm gonna add some cleanup comments for you could address in others PRs.

bcardosolopes · 2024-03-15T03:04:55Z

clang/lib/CIR/CodeGen/CIRDataLayout.h

@@ -30,9 +30,18 @@ class CIRDataLayout {

  // `useABI` is `true` if not using prefered alignment.
  unsigned getAlignment(mlir::Type ty, bool useABI) const {
+    if (llvm::isa<mlir::cir::StructType>(ty)) {


This could be:

if (auto sTy = ty.dyn_cast<mlir::cir::StructType>()) {

bcardosolopes · 2024-03-15T03:05:12Z

clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp

@@ -294,6 +297,11 @@ void CIRRecordLowering::lower(bool nonVirtualBaseType) {
  // TODO: implemented packed structs


Remove this comment?

This PR intends to fix some problems with packed structures support, so the #473 will work. ### Problems Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. ### Why Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. ### Implementation details 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the llvm#473 will work. ### Problems Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. ### Why Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. ### Implementation details 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the #473 will work. Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the #473 will work. Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the llvm#473 will work. ### Problems Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. ### Why Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. ### Implementation details 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the #473 will work. Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the llvm#473 will work. Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the llvm#473 will work. Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the llvm#473 will work. Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

This PR intends to fix some problems with packed structures support, so the #473 will work. Basically, the main problem for the packed structures support is an absence of arbitrary sized integers in CIR. Well, one workaround is to use `mlir::IntegerType` for that, but it's kind of wrong way (please correct me if I'm wrong). Another way is to introduce this type in CIR. So far I suggest this way: instead of arbitrary sized integers we will create an array of bytes for bitfield storages whenever they doesn't fit into the CIR `IntType`. Well, the original codegen creates storages with alignment 8 - so it can be `i24` storage type for instance. Previously, we just created storages that could be represented as CIR `IntType`: 8, 16, 32, 64. And it was working before I came up with a necessity to support packed structures. At first glance it's not a problem - just add `determinePacked` method from the original codegen and that's it. But it turned out that this method _infers_ the fact if a structure is packed or not. It doesn't use the AST attribute for that as one could think - it works with offsets and alignments of fields. Thus, we either need to invent our own way to determine packed structures (which is error prone and maybe not doable at all) or try to use the existing one. Also, we go closer to the original lllvm's data layout in this case. 1) I had to move the lowering details from the `LoweringPrepare` to the `LowerToLLVM`, because it's not possible to do a `load` from the array of bytes to the integer type - and it's ok in llvm dialect. Thus, all the math operations can be expressed without any problems. Basically the most of the diff you see is because of the changes in the lowering. The remaining part is more or less easy to read. 2) There are minor changes in `CIRRecordLayoutBuilder` - as described above, we use may generate an array of bytes as a storage. 3) Some cosmetic changes in `CIRGenExpr` - since we don't want to infer the storage type again and just use the one stored in the `CIRGenBitFieldInfo`. 4) Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops. 5) I removed `bitfield-ops` test - because now the test cases covered there are all in `bitfields.c` and `bitfields.cpp` . So ... This is still a suggestion, though I believe it's a good one. So you are welcome to discuss, suggest another ways to solve the problem and etc.

This PR adds a support for packed structures. Basically, now both `pragma pack(...)` and `__attribute__((aligned(...)))` should work. The only problem is that `getAlignment` is not a total one - I fix only a couple of issues I faced with - for struct types and arrays.

gitoleg added 4 commits February 20, 2024 12:29

[CIR][Codegen] support for packed structures

09d6784

adds sentenial

ea82ef0

added trivial tests

6641ce8

added one more test

e7948d2

gitoleg added 2 commits February 21, 2024 09:06

clang format

ee347ae

Merge remote-tracking branch 'upstream/main' into packed-structures

bc1177d

Merge remote-tracking branch 'upstream/main' into packed-structures

7dc3497

gitoleg mentioned this pull request Feb 29, 2024

[CIR][Codegen][Lowering] Introduce new bitfield layout #487

Merged

Merge remote-tracking branch 'upstream/main' into packed-structures

f7d26b3

bcardosolopes approved these changes Mar 15, 2024

View reviewed changes

bcardosolopes reviewed Mar 15, 2024

View reviewed changes

bcardosolopes merged commit 1612644 into llvm:main Mar 15, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CIR][Codegen] Initial support for packed structures #473

[CIR][Codegen] Initial support for packed structures #473

gitoleg commented Feb 20, 2024

github-actions bot commented Feb 20, 2024 •

edited

Loading

gitoleg commented Feb 21, 2024 •

edited

Loading

gitoleg commented Mar 14, 2024

bcardosolopes left a comment

bcardosolopes Mar 15, 2024

bcardosolopes Mar 15, 2024

		@@ -294,6 +297,11 @@ void CIRRecordLowering::lower(bool nonVirtualBaseType) {
		// TODO: implemented packed structs

[CIR][Codegen] Initial support for packed structures #473

[CIR][Codegen] Initial support for packed structures #473

Conversation

gitoleg commented Feb 20, 2024

github-actions bot commented Feb 20, 2024 • edited Loading

gitoleg commented Feb 21, 2024 • edited Loading

gitoleg commented Mar 14, 2024

bcardosolopes left a comment

Choose a reason for hiding this comment

bcardosolopes Mar 15, 2024

Choose a reason for hiding this comment

bcardosolopes Mar 15, 2024

Choose a reason for hiding this comment

github-actions bot commented Feb 20, 2024 •

edited

Loading

gitoleg commented Feb 21, 2024 •

edited

Loading