-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CIR] Upstream initial support for switch statements #137106
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 6 commits
f1f56e1
b5d56d5
08ad256
6e1e7c3
a8741c5
ad4fd87
7d5a53f
d7dbe0c
25aabcd
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -470,7 +470,8 @@ def StoreOp : CIR_Op<"store", [ | |||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| def ReturnOp : CIR_Op<"return", [ParentOneOf<["FuncOp", "ScopeOp", "IfOp", | ||||||
| "DoWhileOp", "WhileOp", "ForOp"]>, | ||||||
| "SwitchOp", "DoWhileOp","WhileOp", | ||||||
| "ForOp", "CaseOp"]>, | ||||||
| Terminator]> { | ||||||
| let summary = "Return from function"; | ||||||
| let description = [{ | ||||||
|
|
@@ -609,8 +610,9 @@ def ConditionOp : CIR_Op<"condition", [ | |||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| def YieldOp : CIR_Op<"yield", [ReturnLike, Terminator, | ||||||
| ParentOneOf<["IfOp", "ScopeOp", "WhileOp", | ||||||
| "ForOp", "DoWhileOp"]>]> { | ||||||
| ParentOneOf<["IfOp", "ScopeOp", "SwitchOp", | ||||||
| "WhileOp", "ForOp", "CaseOp", | ||||||
| "DoWhileOp"]>]> { | ||||||
| let summary = "Represents the default branching behaviour of a region"; | ||||||
| let description = [{ | ||||||
| The `cir.yield` operation terminates regions on different CIR operations, | ||||||
|
|
@@ -753,6 +755,221 @@ def ScopeOp : CIR_Op<"scope", [ | |||||
| ]; | ||||||
| } | ||||||
|
|
||||||
| //===----------------------------------------------------------------------===// | ||||||
| // SwitchOp | ||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| def CaseOpKind_DT : I32EnumAttrCase<"Default", 1, "default">; | ||||||
| def CaseOpKind_EQ : I32EnumAttrCase<"Equal", 2, "equal">; | ||||||
| def CaseOpKind_AO : I32EnumAttrCase<"Anyof", 3, "anyof">; | ||||||
| def CaseOpKind_RG : I32EnumAttrCase<"Range", 4, "range">; | ||||||
|
|
||||||
| def CaseOpKind : I32EnumAttr< | ||||||
| "CaseOpKind", | ||||||
| "case kind", | ||||||
| [CaseOpKind_DT, CaseOpKind_EQ, CaseOpKind_AO, CaseOpKind_RG]> { | ||||||
| let cppNamespace = "::cir"; | ||||||
| } | ||||||
|
|
||||||
| def CaseOp : CIR_Op<"case", [ | ||||||
| DeclareOpInterfaceMethods<RegionBranchOpInterface>, | ||||||
| RecursivelySpeculatable, AutomaticAllocationScope]> { | ||||||
| let summary = "Case operation"; | ||||||
| let description = [{ | ||||||
| The `cir.case` operation represents a case within a C/C++ switch. | ||||||
| The `cir.case` operation must be in a `cir.switch` operation directly | ||||||
| or indirectly. | ||||||
|
|
||||||
| The `cir.case` have 4 kinds: | ||||||
| - `equal, <constant>`: equality of the second case operand against the | ||||||
| condition. | ||||||
| - `anyof, [constant-list]`: equals to any of the values in a subsequent | ||||||
| following list. | ||||||
| - `range, [lower-bound, upper-bound]`: the condition is within the closed | ||||||
| interval. | ||||||
| - `default`: any other value. | ||||||
|
|
||||||
| Each case region must be explicitly terminated. | ||||||
| }]; | ||||||
|
|
||||||
| let arguments = (ins ArrayAttr:$value, CaseOpKind:$kind); | ||||||
| let regions = (region AnyRegion:$caseRegion); | ||||||
|
|
||||||
| let assemblyFormat = "`(` $kind `,` $value `)` $caseRegion attr-dict"; | ||||||
|
|
||||||
| let skipDefaultBuilders = 1; | ||||||
| let builders = [ | ||||||
| OpBuilder<(ins "mlir::ArrayAttr":$value, | ||||||
| "CaseOpKind":$kind, | ||||||
| "mlir::OpBuilder::InsertPoint &":$insertPoint)> | ||||||
| ]; | ||||||
| } | ||||||
|
|
||||||
| def SwitchOp : CIR_Op<"switch", | ||||||
| [SameVariadicOperandSize, | ||||||
| DeclareOpInterfaceMethods<RegionBranchOpInterface>, | ||||||
| RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments]> { | ||||||
| let summary = "Switch operation"; | ||||||
| let description = [{ | ||||||
| The `cir.switch` operation represents C/C++ switch functionality for | ||||||
| conditionally executing multiple regions of code. The operand to an switch | ||||||
| is an integral condition value. | ||||||
|
|
||||||
| The set of `cir.case` operations and their enclosing `cir.switch` | ||||||
| represents the semantics of a C/C++ switch statement. Users can use | ||||||
| `collectCases(llvm::SmallVector<CaseOp> &cases)` to collect the `cir.case` | ||||||
| operation in the `cir.switch` operation easily. | ||||||
|
|
||||||
| The `cir.case` operations doesn't have to be in the region of `cir.switch` | ||||||
|
||||||
| The `cir.case` operations doesn't have to be in the region of `cir.switch` | |
| The `cir.case` operations don't have to be in the region of `cir.switch` |
This one too.
Outdated
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.
| directly. However, when all the `cir.case` operations lives in the region | |
| directly. However, when all the `cir.case` operations live in the region |
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.
Done
Outdated
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.
| of `cir.switch` directly and there is no other operations except the ending | |
| of `cir.switch` directly and there are no other operations except the ending |
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.
Done
Outdated
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.
| `cir.yield` operation in the region of `cir.switch` directly, we call the | |
| `cir.yield` operation in the region of `cir.switch` directly, we say the |
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.
Done
Outdated
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.
| makes analysis easier to handle the `cir.switch` operation | |
| makes it easier for analyses to handle the `cir.switch` operation |
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.
Done
Outdated
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.
| and makes the boundary to give up pretty clear. | |
| and makes the boundary to give up clear. |
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.
Done
Outdated
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.
Is the blank line and the difference in indentation necessary for this example? I'd be less confused if it was just this:
case 4:
a++;
b++;
case 5:
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.
Done
Outdated
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.
| case 5; | |
| case 5: |
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.
Done
Outdated
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.
| The cir.switch might not be considered "simple" if any of the following is | |
| The cir.switch is not be considered "simple" if any of the following is |
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.
Done
Outdated
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.
| - There are case statements of the switch statement lives in other scopes | |
| - There are case statements of the switch statement that are in a scope |
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.
Done
Outdated
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.
| - There are codes before the first case statement. For example, | |
| - There is any code before the first case statement. For example, |
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.
Done
Outdated
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.
Can you reformat this to fit within 80 characters? Maybe by defining an alias similar to BuilderCallbackRef (as defined in CIRDialect.h)?
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.
Done
Outdated
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.
| void collectCases(llvm::SmallVector<CaseOp> &cases); | |
| void collectCases(llvm::SmallVectorImpl<CaseOp> &cases); |
I believe that using SmallVector directly gets the default size template argument and will cause a copy if the function is called with an object that was constructed with any other size argument. See the note here: https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h
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.
Done
Outdated
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.
| bool isSimpleForm(llvm::SmallVector<CaseOp> &cases); | |
| bool isSimpleForm(llvm::SmallVectorImpl<CaseOp> &cases); |
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.
Done
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.
Sorry I missed this one in my earlier grammar corrections.