New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CIR][Codegen] Adds Stack save-restore ops #346

Merged

bcardosolopes merged 14 commits into llvm:main from gitoleg:stack-save-restore

Dec 21, 2023

Collaborator

gitoleg commented Dec 4, 2023

This PR adds cir.stack_save and cir.stack_restore operations.

sitio-couto reviewed

View reviewed changes

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved

gitoleg changed the title ~~[CIR][Codegen] Ads Stack save-restore ops~~ [CIR][Codegen] Adds Stack save-restore ops

bcardosolopes requested changes

View reviewed changes

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved

clang/include/clang/CIR/Dialect/IR/CIROps.td Show resolved Hide resolved

clang/test/CIR/Lowering/stack-save-restore.c Outdated Show resolved Hide resolved

bcardosolopes reviewed

View reviewed changes

clang/test/CIR/Lowering/stack-save-restore.c Outdated Show resolved Hide resolved

Collaborator Author

gitoleg commented Dec 15, 2023

@bcardosolopes
As we discussed earlier, we want to keep CIR clean and don't emit all this loads/stores/allocas for cir.stack_save and cir.stack_restore and postpoine it until the lowering stage. Latest changes are all about it, so please take a look at LowerToLLVM.cpp. The approach may look messy a little, but I think it's ok for the problem in question. I also added some comments.

Several words about implementation.

cir.stack_save lowering. We insert llvm.alloca before and llvm.store after the llvm.stack_save.
cir.stack_restore lowering. This one is a bit harder. We need to find a memory location where to load from, i.e. the llvm.alloca instruction from above. Assuming, that there is only one cir.stack_save in block, we need to find such llvm.store that has llvm.stack_save and llvm.alloca as operands. Then we insert llvm.load and use its result to create llvm.stack_restore.

bcardosolopes requested changes

View reviewed changes

Member

bcardosolopes left a comment

Inline comment!

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp Outdated

+                mlir::Operation* findAlloca(mlir::Block& blk) const {
+                  mlir::Operation* stackSave;
+                  for (auto& op : blk.getOperations()) {

Member

bcardosolopes Dec 19, 2023

This kinda logic seems a bit overkill just for lowering. I'm having second thoughts about this, perhaps it's just better to go for the initial approach. Let's just emit allocas in CIR like you did before and call it a day. In the future, if we care about optimizing these operations, we can work on that. I apologize for the back and forth!

lanza force-pushed the main branch 2 times, most recently from 05ffb2a to 34fceae Compare

December 20, 2023 07:10

gitoleg added 14 commits

December 20, 2023 14:40


          adds stack save/restore operations

be5baa0


          [CIR][Codegen] adds tests for the stack save/restore

6f6c08c


          adds more detail description

a47a102

wip

8b675db


          adds verifier to the cir.stack_restore

b0a4e61


          typo fix

4abb612


          adds helpers for stack save/restore

347a787


          adds a proper lowering to LLVM dialect

4b13aab


          renaming

6d9923b


          typo fixed

75c9730


          typo fixed

4fd2d31


          fixed some merge artifacts

b05c79b


          back to the beginning

67cbbf0


          minor changes

93751bb

gitoleg force-pushed the stack-save-restore branch from 3d4a336 to 93751bb Compare

December 20, 2023 13:35

Collaborator Author

gitoleg commented Dec 20, 2023

@bcardosolopes done! now there are no extra complications in the PR!

bcardosolopes approved these changes

View reviewed changes

Member

bcardosolopes left a comment

LGTM

bcardosolopes merged commit 5fc351f into llvm:main

4 of 6 checks passed

gitoleg mentioned this pull request

[CIR][CodeGen] Initial variable length array support #398

Merged

lanza pushed a commit that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (#346)

c6422c8

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

bcardosolopes pushed a commit that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (#398)

61fab7a

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in #367 #346 #340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

lanza pushed a commit that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (#346)

3186c8a

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

lanza pushed a commit that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (#398)

e4fee87

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in #367 #346 #340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

eZWALT pushed a commit to eZWALT/clangir that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (llvm#346)

81be6d5

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

eZWALT pushed a commit to eZWALT/clangir that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (llvm#346)

46d09e3

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

eZWALT pushed a commit to eZWALT/clangir that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (llvm#398)

87e5db1

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in llvm#367 llvm#346 llvm#340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

lanza pushed a commit that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (#346)

045b53f

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

lanza pushed a commit that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (#398)

6e1677c

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in #367 #346 #340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

lanza pushed a commit that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (#346)

7c9c457

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

lanza pushed a commit that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (#398)

8e27821

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in #367 #346 #340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

eZWALT pushed a commit to eZWALT/clangir that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (llvm#346)

c86bf5b

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

eZWALT pushed a commit to eZWALT/clangir that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (llvm#398)

efea52f

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in llvm#367 llvm#346 llvm#340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

lanza pushed a commit that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (#346)

6c4bf4c

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

lanza pushed a commit that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (#398)

3f9153d

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in #367 #346 #340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

bruteforceboy pushed a commit to bruteforceboy/clangir that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (llvm#346)

18604eb

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

bruteforceboy pushed a commit to bruteforceboy/clangir that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (llvm#398)

ccd27e6

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in llvm#367 llvm#346 llvm#340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (llvm#346)

abadf11

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (llvm#398)

33b9e2e

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in llvm#367 llvm#346 llvm#340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

keryell pushed a commit to keryell/clangir that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (llvm#346)

9ac3c22

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

keryell pushed a commit to keryell/clangir that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (llvm#398)

fd6b630

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in llvm#367 llvm#346 llvm#340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

lanza pushed a commit that referenced this pull request


          [CIR][Codegen] Adds Stack save-restore ops (#346)

0060ea7

This PR adds `cir.stack_save` and `cir.stack_restore` operations.

lanza pushed a commit that referenced this pull request


          [CIR][CodeGen] Initial variable length array support (#398)

80d9276

This is a first PR for variable length array support. There are one (or
more :) ) ahead.

Basically, we already did lot's of preliminary job in order to land VLA
in CIR in #367 #346 #340. So now we add initial VLA support itself.

Most of the changes are taken from the original codegen, so there is
nothing to be scary of)

I added just one test, and basically that's all we can test right now.
Later, I will add more, from the original codegen tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet