-
Notifications
You must be signed in to change notification settings - Fork 147
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
Overflow detection in simulation #777
Conversation
dc1c6c7
to
5abf580
Compare
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.
As well should have tests
compilers/concrete-compiler/compiler/lib/Runtime/simulation.cpp
Outdated
Show resolved
Hide resolved
compilers/concrete-compiler/compiler/lib/Conversion/SimulateTFHE/SimulateTFHE.cpp
Show resolved
Hide resolved
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.
Also, we need to check for errors after table lookups as they can result in overflows (imagine, [1, 2, 3000, 4] table and 2
is not observed in evaluation, but happened in simulation)
compilers/concrete-compiler/compiler/lib/Runtime/simulation.cpp
Outdated
Show resolved
Hide resolved
e0a0ee9
to
32f0998
Compare
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.
On my previous review I totally forgot the signed integer case, actually the encoding differs from unsigned and signed integers. On unsigned integers we let the padding bit clean, but on signed integers we use the padding bit as the traditional sign bit to let the leveled signed operations works natively then we rescale before tlu.
So we should check overflow in different way on signed (against 64bits) and unsigned (against 63bits). One of the issue is that the signed information is not available at TFHE level. Let's live discuss of that.
compilers/concrete-compiler/compiler/lib/Conversion/SimulateTFHE/SimulateTFHE.cpp
Outdated
Show resolved
Hide resolved
compilers/concrete-compiler/compiler/lib/Conversion/SimulateTFHE/SimulateTFHE.cpp
Outdated
Show resolved
Hide resolved
ea25a58
to
792e5cf
Compare
compilers/concrete-compiler/compiler/lib/Conversion/FHEToTFHEScalar/FHEToTFHEScalar.cpp
Outdated
Show resolved
Hide resolved
compilers/concrete-compiler/compiler/lib/Runtime/simulation.cpp
Outdated
Show resolved
Hide resolved
compilers/concrete-compiler/compiler/lib/Runtime/simulation.cpp
Outdated
Show resolved
Hide resolved
3c4f5e2
to
c2ec1a4
Compare
compilers/concrete-compiler/compiler/lib/Runtime/simulation.cpp
Outdated
Show resolved
Hide resolved
compilers/concrete-compiler/compiler/lib/Runtime/simulation.cpp
Outdated
Show resolved
Hide resolved
0ea44f2
to
463a1b3
Compare
No description provided.