-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CIR] Add if statement support #134333
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
[CIR] Add if statement support #134333
Changes from all commits
a91edc3
eba4924
25dff99
c997d6c
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 |
|---|---|---|
|
|
@@ -135,6 +135,71 @@ mlir::Location CIRGenFunction::getLoc(mlir::Location lhs, mlir::Location rhs) { | |
| return mlir::FusedLoc::get(locs, metadata, &getMLIRContext()); | ||
| } | ||
|
|
||
| bool CIRGenFunction::containsLabel(const Stmt *s, bool ignoreCaseStmts) { | ||
| // Null statement, not a label! | ||
| if (!s) | ||
| return false; | ||
|
|
||
| // If this is a label, we have to emit the code, consider something like: | ||
| // if (0) { ... foo: bar(); } goto foo; | ||
| // | ||
| // TODO: If anyone cared, we could track __label__'s, since we know that you | ||
| // can't jump to one from outside their declared region. | ||
| if (isa<LabelStmt>(s)) | ||
| return true; | ||
|
|
||
| // If this is a case/default statement, and we haven't seen a switch, we | ||
| // have to emit the code. | ||
| if (isa<SwitchCase>(s) && !ignoreCaseStmts) | ||
| return true; | ||
|
|
||
| // If this is a switch statement, we want to ignore case statements when we | ||
| // recursively process the sub-statements of the switch. If we haven't | ||
| // encountered a switch statement, we treat case statements like labels, but | ||
| // if we are processing a switch statement, case statements are expected. | ||
| if (isa<SwitchStmt>(s)) | ||
|
Collaborator
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 don't think this is necessary? So the comment isn't really accurate
Member
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. This matches OG and it's probably being conservative, I wouldn't go about changing it right now (at least not until this is also tested / changed in OG as well).
Collaborator
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. Its actually being the inverse of conservative, isn't it? It is going to end up skipping blocks that it shouldn't, causing compile-issues.
Contributor
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 dug into this a bit more and I think the code here is correct. This function is calling itself recursively to walk through all the sub-statements in the original statement. If we hit a This is basically happening in the context of determining whether or not we can constant-fold the statement. I put together an experiment to show how the classic codegen handles labels in a switch statement. https://godbolt.org/z/dY14fWezT If there is a label in the middle of a Note that the clang IR incubator doesn't fold either case, but it does if the |
||
| ignoreCaseStmts = true; | ||
|
|
||
| // Scan subexpressions for verboten labels. | ||
| return std::any_of(s->child_begin(), s->child_end(), | ||
| [=](const Stmt *subStmt) { | ||
| return containsLabel(subStmt, ignoreCaseStmts); | ||
| }); | ||
| } | ||
|
|
||
| /// If the specified expression does not fold to a constant, or if it does but | ||
| /// contains a label, return false. If it constant folds return true and set | ||
| /// the boolean result in Result. | ||
| bool CIRGenFunction::constantFoldsToBool(const Expr *cond, bool &resultBool, | ||
| bool allowLabels) { | ||
| llvm::APSInt resultInt; | ||
| if (!constantFoldsToSimpleInteger(cond, resultInt, allowLabels)) | ||
| return false; | ||
|
|
||
| resultBool = resultInt.getBoolValue(); | ||
| return true; | ||
| } | ||
|
|
||
| /// If the specified expression does not fold to a constant, or if it does | ||
| /// fold but contains a label, return false. If it constant folds, return | ||
| /// true and set the folded value. | ||
| bool CIRGenFunction::constantFoldsToSimpleInteger(const Expr *cond, | ||
| llvm::APSInt &resultInt, | ||
| bool allowLabels) { | ||
| // FIXME: Rename and handle conversion of other evaluatable things | ||
| // to bool. | ||
| Expr::EvalResult result; | ||
| if (!cond->EvaluateAsInt(result, getContext())) | ||
| return false; // Not foldable, not integer or not fully evaluatable. | ||
|
|
||
| llvm::APSInt intValue = result.Val.getInt(); | ||
| if (!allowLabels && containsLabel(cond)) | ||
| return false; // Contains a label. | ||
|
|
||
| resultInt = intValue; | ||
| return true; | ||
| } | ||
|
|
||
| void CIRGenFunction::emitAndUpdateRetAlloca(QualType type, mlir::Location loc, | ||
| CharUnits alignment) { | ||
| if (!type->isVoidType()) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.