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

Ite predication #9

Merged
merged 2 commits into from
Aug 8, 2018
Merged

Conversation

DIJamner
Copy link
Contributor

Changes the semantics of ite expressions to evaluate their branches before reducing the condition. Also adds last rule necessary to prove progress and preservation for expression evaluation. Note that this pull request is on top of pull request #8. This pull request should only be merged if it is determined that this behavior for ite is desirable.

@ivg
Copy link
Member

ivg commented Jul 25, 2018

Could you please remove the badc474 commit from this branch as it is already in the master branch?

Copy link
Member

@ivg ivg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using the if <cond> then <exp1> else <exp2> syntax that vocalizes that this is a control-flow expression and brings all the common intuition that the evaluation of the branches shall not be strict.

So in order not to bring the wrath of angriness upon us, I'm suggesting changing the concrete syntax as well :)

I've reviewed different languages and systems, to understand which syntax will bring the right intuition.

  • c ? x : y is used by Verilog to define multiplexers, but it is also defined C and most people will think in terms of C, rather than Verilog.

  • x when c else y is used in VHDL for the same purpose, but it resembles a lot Pythonish x if c else y, so it bears the same wrong intuition. Also, it is verbose.

  • mux c x y is used by Hardcaml, however, it sounds to low-level (despite the fact, that reflects the exact semantics)

  • ite c x y is used by SMT-LIB where it is defined as a function in the Core logic.

While mux looks like the right solution, I personally don't like its low-levelness. I believe that majority of BAP users reason more in terms of SMT-LIB and mathematical logic, rather than combinatorial logic.

So, if nobody have any objections, could you please change it to ite c x y syntax.

Dustin Jamner added 2 commits July 25, 2018 18:16
@DIJamner
Copy link
Contributor Author

The ite syntax sounds good to me. I've made the change and cleaned up the branch.

@ivg
Copy link
Member

ivg commented Jul 25, 2018

thanks!
I will keep it open for a couple of days to give others an opportunity to express their opinions.

@ethan42
Copy link

ethan42 commented Jul 25, 2018

Looks like a very reasonable PR 👍

@ivg ivg self-assigned this Jul 26, 2018
ethan42 added a commit to ethan42/bap that referenced this pull request Jul 27, 2018
The semantics of ite evaluation in Primus follow the latest updates
in the BIL specification:

   BinaryAnalysisPlatform/bil#9

The extra option in the BIL normalization function enables the
user to preserve ite if that is desirable.
ethan42 added a commit to ethan42/bap that referenced this pull request Aug 1, 2018
The semantics of ite evaluation in Primus follow the latest updates
in the BIL specification:

   BinaryAnalysisPlatform/bil#9

The extra option in the BIL normalization function enables the
user to preserve ite if that is desirable.
ivg pushed a commit to BinaryAnalysisPlatform/bap that referenced this pull request Aug 7, 2018
…#850)

The semantics of ite evaluation in Primus follow the latest updates
in the BIL specification:

   BinaryAnalysisPlatform/bil#9

The extra option in the BIL normalization function enables the
user to preserve ite if that is desirable.
@ivg ivg merged commit 7a9d451 into BinaryAnalysisPlatform:master Aug 8, 2018
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.

3 participants