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

gitoleg · 2024-02-29T15:08:50Z

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

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.
There are minor changes in CIRRecordLayoutBuilder - as described above, we use may generate an array of bytes as a storage.
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.
Helpers are introduced in the lowering - but nothing hard - just shifts and logical ops.
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.

github-actions · 2024-02-29T15:12:13Z

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

bcardosolopes

Hi Oleg, thanks for improving the situation here and thanks for the detailed explanation on why this is necessary - very insightful. LGTM with some minor nitpicks:

Can you add some comments to the CIRGen parts where we now do differently from traditional CodeGen? Doesn't need to be a lot of content, but more something like // This is different from LLVM traditional codegen because CIRGen uses arrays of bytes instead of arbitrary-sized integers.
I also noticed both get_bitfield and set_bitfield are missing the MemRead and MemWrite markers, can you add that in a follow up PR?
Fix the remaining merge conflict

clang/lib/CIR/CodeGen/CIRGenBuilder.h

clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp

This PR adds MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in #487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in #487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in llvm#487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in #487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in #487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in llvm#487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in #487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in llvm#487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in llvm#487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in llvm#487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

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 MemRead/MemWrite markers to the `GetBitfieldOp` and `SetBitfieldOp` (as discussed in #487) Also, minor renaming in the `SetBitfieldOp` --------- Co-authored-by: Bruno Cardoso Lopes <bcardosolopes@users.noreply.github.com>

gitoleg added 23 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

clang format

ee347ae

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

bc1177d

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

7dc3497

omg, wip

d6451db

looks like something starts to work

cd06ee8

helpers, refactoring

27dd7b3

refactoring

4405c76

wip

b2fc073

Merge remote-tracking branch 'upstream/main' into new-bitfield-layout

37fc173

wip

7936c4f

try this one: both intType and ByteArrayType supported

d3586bf

work on tests

45d43fd

fixed all tests

8d86d82

use volatile for lowering

3f2653a

removed bitfield-ops.c

11db130

cleaned the code

0c789d0

bug fixed

55e041f

returns the SetBitfield result

35c6583

fixes minor issues

6125f95

gitoleg changed the title ~~New bitfield layout~~ [CIR][Codegen][Lowering] Introduce new bitfield layout Feb 29, 2024

gitoleg added 4 commits February 29, 2024 18:17

clang-format ...

e44431d

merg

8b0b837

clang-format ...

68971d6

forgotten return val

859141d

bcardosolopes approved these changes Mar 6, 2024

View reviewed changes

clang/lib/CIR/CodeGen/CIRGenBuilder.h Outdated Show resolved Hide resolved

clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp Show resolved Hide resolved

gitoleg added 6 commits March 7, 2024 13:50

Merge remote-tracking branch 'upstream/main' into new-bitfield-layout

8840bb6

refactoring

2fbbb92

Merge remote-tracking branch 'upstream/main' into new-bitfield-layout

4700574

clang-format ...

c35d31a

minor fix

dbd0a46

bug fixed

9097afd

bcardosolopes merged commit 278c391 into llvm:main Mar 12, 2024
6 checks passed

This was referenced Mar 13, 2024

[CIR] adds MemRead/MemWrite markers to bitfield ops #507

Merged

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

gitoleg commented Feb 29, 2024 •

edited

Loading

github-actions bot commented Feb 29, 2024 •

edited

Loading

bcardosolopes left a comment

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

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

Conversation

gitoleg commented Feb 29, 2024 • edited Loading

Problems

Why

Implementation details

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

bcardosolopes left a comment

Choose a reason for hiding this comment

gitoleg commented Feb 29, 2024 •

edited

Loading

github-actions bot commented Feb 29, 2024 •

edited

Loading