-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[CIR] Upstream StackSave and StackRestoreOp #136426
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.
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
Changes from 2 commits
91a6937
015a2a9
88ffbd9
bb865d2
1ecbfe8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1419,6 +1419,50 @@ def CallOp : CIR_CallOpBase<"call", [NoRegionArguments]> { | |||||
}]>]; | ||||||
} | ||||||
|
||||||
//===----------------------------------------------------------------------===// | ||||||
// StackSave & StackRestoreOp | ||||||
//===----------------------------------------------------------------------===// | ||||||
|
||||||
def StackSaveOp : CIR_Op<"stack_save"> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: These should probably mention that operations correspond and mirror semantics of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Also any reason not to mirror There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I found that we use the same name pattern in general, for example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mmmmm, I think that llvm in general uses the same style without _ for example getElemenetPtr, but in CIR dialect we use different style, not sure if it make sense to rename stack_save to stacksave or not, what do you think @xlauko 🤔 Edit: Another example we have is insertvalue and insert_value There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, probably generic question on @bcardosolopes @andykaylor what should be the policy? I don't mind either. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for bringing up the consistency angle here. We have been telling people to use |
||||||
let summary = "remembers the current state of the function stack"; | ||||||
let description = [{ | ||||||
Remembers the current state of the function stack. Returns a pointer | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
that later can be passed into cir.stack_restore. | ||||||
Useful for implementing language features like variable length arrays. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
This operation is correspond to LLVM intrinsic `stacksave`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
```mlir | ||||||
%0 = cir.stack_save : <!u8i> | ||||||
``` | ||||||
}]; | ||||||
|
||||||
let results = (outs CIR_PointerType:$result); | ||||||
let assemblyFormat = "attr-dict `:` qualified(type($result))"; | ||||||
} | ||||||
|
||||||
def StackRestoreOp : CIR_Op<"stack_restore"> { | ||||||
let summary = "restores the state of the function stack"; | ||||||
let description = [{ | ||||||
Restore the state of the function stack to the state it was | ||||||
in when the corresponding cir.stack_save executed. | ||||||
Useful for implementing language features like variable length arrays. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
This operation is correspond to LLVM intrinsic `stackrestore`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
```mlir | ||||||
%0 = cir.alloca !cir.ptr<!u8i>, !cir.ptr<!cir.ptr<!u8i>>, ["saved_stack"] {alignment = 8 : i64} | ||||||
%1 = cir.stack_save : <!u8i> | ||||||
cir.store %1, %0 : !cir.ptr<!u8i>, !cir.ptr<!cir.ptr<!u8i>> | ||||||
%2 = cir.load %0 : !cir.ptr<!cir.ptr<!u8i>>, !cir.ptr<!u8i> | ||||||
cir.stack_restore %2 : !cir.ptr<!u8i> | ||||||
``` | ||||||
}]; | ||||||
|
||||||
let arguments = (ins CIR_PointerType:$ptr); | ||||||
let assemblyFormat = "$ptr attr-dict `:` qualified(type($ptr))"; | ||||||
} | ||||||
|
||||||
//===----------------------------------------------------------------------===// | ||||||
// UnreachableOp | ||||||
//===----------------------------------------------------------------------===// | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
// Test the CIR operations can parse and print correctly (roundtrip) | ||
|
||
// RUN: cir-opt %s | cir-opt | FileCheck %s | ||
|
||
!u8i = !cir.int<u, 8> | ||
|
||
module { | ||
cir.func @stack_save_restore() { | ||
%0 = cir.stack_save : !cir.ptr<!u8i> | ||
cir.stack_restore %0 : !cir.ptr<!u8i> | ||
cir.return | ||
} | ||
} | ||
|
||
//CHECK: module { | ||
|
||
//CHECK-NEXT: cir.func @stack_save_restore() { | ||
//CHECK-NEXT: %0 = cir.stack_save : !cir.ptr<!u8i> | ||
//CHECK-NEXT: cir.stack_restore %0 : !cir.ptr<!u8i> | ||
//CHECK-NEXT: cir.return | ||
//CHECK-NEXT: } | ||
|
||
//CHECK-NEXT: } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// RUN: cir-opt %s -cir-to-llvm -o - | FileCheck %s -check-prefix=MLIR | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you also add a RUN line to lower this to LLVM IR? |
||
|
||
!u8i = !cir.int<u, 8> | ||
|
||
module { | ||
cir.func @stack_save() { | ||
%0 = cir.stack_save : !cir.ptr<!u8i> | ||
cir.stack_restore %0 : !cir.ptr<!u8i> | ||
cir.return | ||
} | ||
} | ||
|
||
// MLIR: module { | ||
// MLIR-NEXT: llvm.func @stack_save | ||
// MLIR-NEXT: %0 = llvm.intr.stacksave : !llvm.ptr | ||
// MLIR-NEXT: llvm.intr.stackrestore %0 : !llvm.ptr | ||
// MLIR-NEXT: llvm.return | ||
// MLIR-NEXT: } | ||
// MLIR-NEXT: } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, though I'm not sure why there are combined.