Move definition of can-be-constant-propagated to a central location#2221
Move definition of can-be-constant-propagated to a central location#2221tautschnig merged 1 commit intodiffblue:developfrom
Conversation
tautschnig
requested review from
kroening,
peterschrammel and
smowton
as code owners
| } | ||
| else if(expr.id()==ID_with) | ||
| { | ||
| // this is bad |
There was a problem hiding this comment.
This is probably not your code, but it would be good to give an idea of 'what didn't work here' in place of a commented piece of code.
There was a problem hiding this comment.
Indeed, let me try to dig this up.
There was a problem hiding this comment.
(But it might take a while to do so.)
There was a problem hiding this comment.
Throughout the back-end there are hints that the array theory used to do Strange Things. At one point I (tried to) patch a rewrite that converted with to ITE.
martin-cs
left a comment
martin-cs
left a comment
There was a problem hiding this comment.
A really good review strategy is to find simple refactorings or moving the code around and ask for complex semantic changes!
Happy with this as a refactor but think some of the questions are worth discussing.
| { | ||
| if(expr.id()==ID_mult) | ||
| { | ||
| // propagate stuff with sizeof in it |
There was a problem hiding this comment.
Why is this only the case for multiply?
(Again, I realise that this is not new code.)
| } | ||
| else if(expr.id()==ID_with) | ||
| { | ||
| // this is bad |
There was a problem hiding this comment.
Throughout the back-end there are hints that the array theory used to do Strange Things. At one point I (tried to) patch a rewrite that converted with to ITE.
| return is_constant(expr.op0()); | ||
| } | ||
| else if( | ||
| expr.id() == ID_plus || expr.id() == ID_mult || expr.id() == ID_array || |
There was a problem hiding this comment.
What about divide, floating-point ops, etc. Do we have any exprt's that cannot be constant folded if all arguments are constant? If we do; should we?
I realise that constant folding and expression forwarding in symex is more involved than just "is it constant" but...
tautschnig
force-pushed
the
is-constantt
branch
from
75a9a24 to
fd339a7
Compare
|
@kroening Would you be able to respond to some of the questions above? |
tautschnig
force-pushed
the
is-constantt
branch
from
fd339a7 to
8d24d2f
Compare
allredj
left a comment
allredj
left a comment
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: 8d24d2f).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86785268
peterschrammel
left a comment
peterschrammel
left a comment
There was a problem hiding this comment.
I suggest to turn the questions into issues instead of delaying this further.
This is re-usable, for example for the constant-propagation data-flow analysis.
tautschnig
force-pushed
the
is-constantt
branch
from
8d24d2f to
be11b36
Compare
allredj
left a comment
allredj
left a comment
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: be11b36).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86839351
6 participants
This is re-usable, for example for the constant-propagation data-flow analysis.
This is factored out from #2132.