-
Notifications
You must be signed in to change notification settings - Fork 105
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
Added explicit Location parameter to various constructors for Expr and Stmt #139
Added explicit Location parameter to various constructors for Expr and Stmt #139
Conversation
@@ -237,7 +237,7 @@ impl ToIrep for ExprValue { | |||
ExprValue::SelfOp { op, e } => side_effect_irep(op.to_irep_id(), vec![e.to_irep(mm)]), | |||
ExprValue::StatementExpression { statements: ops } => side_effect_irep( | |||
IrepId::StatementExpression, | |||
vec![Stmt::block(ops.to_vec()).to_irep(mm)], | |||
vec![Stmt::block(ops.to_vec(), Location::none()).to_irep(mm)], |
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 there no location we can use here?
]) | ||
Stmt::block( | ||
vec![ | ||
self.codegen_expr_to_place(&p, func_expr.call(fargs)) |
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.
This is maybe for another PR, but codegen_expr_to_place
should really just take a location
Stmt::goto( | ||
self.current_fn().labels()[targets[jmp2].index()].clone(), | ||
self.current_fn().labels()[targets[jmp].index()].clone(), |
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.
I was going to ask why this changed from jmp2
to jmp
but I think its an artifact of the way github splits the lines.
Added Location to Expr::assign Added Location to Expr::as_stmt Added Location to Stmt::atomic_block Added Location to Stmt::block Fixed move errors. Fixed merge conflicts.
f2073fa
to
7159abc
Compare
When you merge this, make sure to cleanup the commit comment. |
Add SimdArray trait and safe gather/scatter API (rust-lang/portable-simd#139) This PR has four parts, without which it doesn't make a lot of sense: - The introduction of the SimdArray trait for abstraction over vectors. - The implementation of private vector-of-pointers types. - Using these to allow constructing vectors with SimdArray::gather_{or, or_default, select}. - Using these to allow writing vectors using SimdArray::scatter{,_select}.
Combine LanesAtMost32 and SimdArray into a single trait "Vector" Attempts to fix some unresolved questions in model-checking#139 regarding `SimdArray` having a generic parameter. In particular, this made it not appropriate for replacing `LanesAtMost32`. Additionally, it made it impossible to use in a context where you otherwise don't know the lane count, e.g. `impl Vector`. An unfortunate side effect of this change is that scatter/gather no longer work in the trait (nor does anything else that references the lane count in a type. This requires the super-unstable `const_evaluatable_checked` feature). I also threw in the change from `as_slice` to `as_array` as discussed in zulip, and fixes #51.
Description of changes:
Currently: We infer location information from children of a node in a few constructors, rather than passing it as an explicit argument.
After update: These constructors require explicit passing of location. All calls to these constructors have been updated, and anywhere where location was not readily available, we pass
Location::none()
.See #136 for instances of missing location information.
Resolved issues:
Call-outs:
Testing:
How is this change tested? Existing regression suite.
Is this a refactor change? Yes.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.