-
Notifications
You must be signed in to change notification settings - Fork 108
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
Optionally encode floating point operations as uninterpreted functions #1057
Conversation
so my (now former) student Zhengyang has been using floats-as-UF in his Alive-based superoptimizer. @zhengyang92 do you have feedback on this? |
I'd consider this ready for review now. As the diff is quite large, I'd recommend looking at the first three commits individually and then looking at the diff between the third and last commit. The main reason for the large diff is that implementing the if (is_uf_float()) {
<create UF>
} else {
<existing code>
} |
@nunoplopes Do you have any feedback on this? |
Sorry for the delay. I still have a few bugs to fix on my queue. I'll get back to this afterwards. |
ir/instr.cpp
Outdated
} | ||
}; | ||
|
||
fast_math_flag(FastMathFlags::NNaN, "nnan"); |
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 don't understand this part. Also, where are the other flags?
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.
It handles flags which may result in the operation returning poison. As far as I can tell the only flags which can result in poison are nnan
and ninf
. They are handled by creating additional uninterpreted functions op.np_nnan(x, y): bool
and op.np_ninf(x, y): bool
(in the binary case) whose results are added to the non_poison
and expression.
The nsz
flag is not relevant as in uf-float mode we do not really have a zero value.
I don't think I entirely understand how the remaining fast math flags are handled in the existing code, though it looks like the result of the operation is wrapped in a function call to another UF.
auto value = expr::mkUF(name, arg_values, res); | ||
if (is_commutative) { | ||
assert(args.size() == 2); | ||
value = value & expr::mkUF(name, {arg_values[1], arg_values[0]}, res); |
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 don't think this is a sound over-approximation. Do you need commutativity in your examples?
Would an axiom stating that forall x, y . fadd(x, y) = fadd(y, x)
suffice?
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 encoding comes from "SMT-based Translation Validation for Machine Learning Compiler" by Seongwon Bang, Seunghyeon Nam, Inwhan Chun, Ho Young Jhoo, and Juneyoung Lee. The idea is to encode commutativity as
where
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 added a comment referencing the paper.
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 proof is very vague and doesn't show equisatisfiability clearly.
Why not use x <= y ? f(x,y) : f(y, x)
?
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 would prefer to keep the current encoding, as I know that it works well on our test cases. While I could try other encodings, that might of course take some time. So here is my attempt at constructing a proof. It is of course inspired by the proof given in the supplementary material, but uses a simpler construction for
Please correct me if I am wrong, but since we are already over approximating, we only need to prove that for all commutative functions
(in the context of SMT solving this ensures that no model/counterexample can get lost.)
Proof: Let
Let
By commutativity of
By
Thus we have shown that
I was just thinking about this again and I think there's a better approach, which is much less invasive. |
I played a bit with the idea, but I was unable to achieve the same performance as when implementing the feature in instr.cpp. I used the I tried a few different things, but here are the most successful approaches I could come up with.
Here are the results. I measured the total runtime of Here is a chart that shows only the approximations. So at least on this benchmark the approach taken by this PR has a clear advantage over inserting the uninterpreted functions in I think that the reason for this difference are the other operations applied Regarding the remaining fast math flags: Implementing the same logic as in |
Here is the relevant part of the diff for diff --git a/smt/expr.cpp b/smt/expr.cpp
index 70353e8a..0af924a2 100644
--- a/smt/expr.cpp
+++ b/smt/expr.cpp
@@ -1205,7 +1205,8 @@ expr expr::fadd(const expr &rhs, const expr &rm) const {
expr expr::fsub(const expr &rhs, const expr &rm) const {
C(rhs, rm);
- return simplify_const(Z3_mk_fpa_sub(ctx(), rm(), ast(), rhs()), *this, rhs);
+ //return simplify_const(Z3_mk_fpa_sub(ctx(), rm(), ast(), rhs()), *this, rhs);
+ return expr::mkUF("fsub", {*this, rhs}, *this);
}
expr expr::fmul(const expr &rhs, const expr &rm) const {
@@ -1282,7 +1283,8 @@ expr expr::foge(const expr &rhs) const {
}
expr expr::folt(const expr &rhs) const {
- return binop_fold(rhs, Z3_mk_fpa_lt);
+ //return binop_fold(rhs, Z3_mk_fpa_lt);
+ return expr::mkUF("flt", {*this, rhs}, true);
}
expr expr::fole(const expr &rhs) const {
@@ -1310,7 +1312,8 @@ expr expr::fuge(const expr &rhs) const {
}
expr expr::fult(const expr &rhs) const {
- return funo(rhs) || binop_fold(rhs, Z3_mk_fpa_lt);
+ //return funo(rhs) || binop_fold(rhs, Z3_mk_fpa_lt);
+ return funo(rhs) || expr::mkUF("flt", {*this, rhs}, true);
}
expr expr::fule(const expr &rhs) const { Here is the relevant part of the diff for diff --git a/smt/expr.cpp b/smt/expr.cpp
index 70353e8a..729c8ac3 100644
--- a/smt/expr.cpp
+++ b/smt/expr.cpp
@@ -1205,7 +1205,8 @@ expr expr::fadd(const expr &rhs, const expr &rm) const {
expr expr::fsub(const expr &rhs, const expr &rm) const {
C(rhs, rm);
- return simplify_const(Z3_mk_fpa_sub(ctx(), rm(), ast(), rhs()), *this, rhs);
+ //return simplify_const(Z3_mk_fpa_sub(ctx(), rm(), ast(), rhs()), *this, rhs);
+ return expr::mkUF("fsub", {float2BV(), rhs.float2BV()}, float2BV()).BV2float(rhs);
}
expr expr::fmul(const expr &rhs, const expr &rm) const {
@@ -1282,7 +1283,8 @@ expr expr::foge(const expr &rhs) const {
}
expr expr::folt(const expr &rhs) const {
- return binop_fold(rhs, Z3_mk_fpa_lt);
+ //return binop_fold(rhs, Z3_mk_fpa_lt);
+ return expr::mkUF("flt", {float2BV(), rhs.float2BV()}, true);
}
expr expr::fole(const expr &rhs) const {
@@ -1310,7 +1312,8 @@ expr expr::fuge(const expr &rhs) const {
}
expr expr::fult(const expr &rhs) const {
- return funo(rhs) || binop_fold(rhs, Z3_mk_fpa_lt);
+ //return funo(rhs) || binop_fold(rhs, Z3_mk_fpa_lt);
+ return funo(rhs) || expr::mkUF("flt", {float2BV(), rhs.float2BV()}, true);
}
expr expr::fule(const expr &rhs) const { |
I think those results are very encouraging and really suggest we should go the expr.cpp way. This solution is superior in terms of maintainability. And getting rid of floats altogether should close the performance gap. |
It would still require some changes in |
I don't remember of cases that are not 1-to-1 mapping with Z3. Do you have an example in mind? (btw, I'm taking off for vacations tmr; I'm back in September) |
Looking through the source code I found the following cases:
For the
Have a nice vacation! Edit: I will be on vacation until September 23. |
Closing this as we don't want to have to duplicate semantics. A solution in smt/expr.cpp would avoid that. |
In our ongoing work with alive2, we often encountered cases where floating point operations remain largely unchanged while other parts of the function differ. Since, Z3's floating point theory is quite slow I am looking at using uninterpreted functions to (over) approximate these floating point operations.
This PR adds a new command line option
--uf-float
that encodes all floating point operations using uninterpreted functions. The encoding is a conservative approximation, meaning that if a counter-example for the approximation is found, it is not necessarily a valid counter example.Commutativity for
FPBinOp
andFCmp
is encoded using the encoding proposed by Seongwon Bang, Seunghyeon Nam, Inwhan Chun, Ho Young Jhoo, and Juneyoung Lee in "SMT-based Translation Validation for Machine Learning Compiler.". It does not use the other parts of the encoding. It might be interesting to encode properties other than commutativity as well though.I decided to open this PR as a draft, to see if there is interest in upstreaming this feature and get some feedback on the direction.
Related: #916