Skip to content
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

Fix sign handling of alias types for binary operators and casts #410

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andidr
Copy link
Contributor

@andidr andidr commented May 27, 2024

The generated IR for some of the AST nodes handled by MLIRScanner::VisitBinaryOperator() and
MLIRScanner::VisitCastExpr() depends on the sign of the operand or result types. Some of these checks attempt to directly cast the result of Expr::getType() to BuiltinType and then check if the resulting QualType is a signed or unsigned integer type. If the cast fails, the type is assumed to be signed.

However, the type being checked may not directly be a builtin type (e.g., it may be an alias for an unsigned type wrapped in a TypedefType) and the default assumption may not hold.

Examining the canonical type retrieved via
QualType::getCanonicalType(), as implemented by this change, solves the issue.

The generated IR for some of the AST nodes handled by
`MLIRScanner::VisitBinaryOperator()` and
`MLIRScanner::VisitCastExpr()` depends on the sign of the operand or
result types. Some of these checks attempt to directly cast the result
of `Expr::getType()` to `BuiltinType` and then check if the resulting
`QualType` is a signed or unsigned integer type. If the cast fails,
the type is assumed to be signed.

However, the type being checked may not directly be a builtin type
(e.g., it may be an alias for an unsigned type wrapped in a
`TypedefType`) and the default assumption may not hold.

Examining the canonical type retrieved via
`QualType::getCanonicalType()`, as implemented by this change, solves
the issue.
@andidr
Copy link
Contributor Author

andidr commented Jul 31, 2024

Kindly asking if anyone is in for a review of this PR. Considering the contributions to the affected files, maybe @wsmoses?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant