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.

Sign up for GitHub

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

Jump to bottom

[Code Generator] Simplify Expressions #318

Merged
merged 49 commits into from
Sep 21, 2023
Merged

[Code Generator] Simplify Expressions #318

merged 49 commits into from
Sep 21, 2023

Conversation

github-actions[bot]
Copy link
Contributor

closes #25

@github-actions github-actions bot added feature-request New feature or request priority-low Low priority issue labels Aug 17, 2023
@steffenenders steffenenders requested a review from blattm August 24, 2023 08:54
@rihi rihi force-pushed the issue-25-_Code_Generator_Simplify_Expressions branch from 721a962 to 58ef1af Compare August 24, 2023 11:20
@rihi rihi marked this pull request as ready for review August 24, 2023 11:29
blattm
blattm reviewed Sep 6, 2023
View reviewed changes
Copy link
Collaborator

@blattm blattm left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! Here is the first part of the review.

decompiler/pipeline/controlflowanalysis/expression_simplification/modification.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/modification.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/modification.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification_rules.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification_rules.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification_rules.py Outdated Show resolved Hide resolved
decompiler/structures/visitors/substitute_visitor.py Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/modification.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/modification.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/modification.py Outdated Show resolved Hide resolved
rihi and others added 19 commits September 6, 2023 18:22
@rihi @blattm
Add documentation to visit_mem_phi in substitute_visitor.py
49fe49d 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi
Add docstring to modification.py/normalize_int(int,int,bool)
db813cf
@rihi
Rename modification.py/FOLDABLE_CONSTANTS to FOLDABLE_OPERATIONS
509298e
@rihi
Rename modification.py/_FOLD_HANDLER to _OPERATION_TO_FOLD_FUNCTION
d577ad9
@rihi
Move _simplify_instructions and _simplify_instructions_with_rule_set …
7f8f1d1 
…into _ExpressionSimplificationRulesBase
@rihi
Extract code from _simplify_instructions_with_rule_set to _simplify_i…
e8992dc 
…nstruction_with_rule
@rihi
Improve documentation in default.json for expression simplification m…
69060a9 
…ax_iterations
@rihi
Add documentation to substitute_visitor.py
3111578
@rihi
Add additional test cases to test_substitute_visitor.py
4e3cd71 
- Assignment lhs
- UnaryOperation
- UnaryOperation ArrayInfo
- Phi
@rihi
Improve readability of _visit_operation(self,Operation) in substitute…
a9bbe64 
…_visitor.py
@rihi
Improve readability of visit_unary_operation(self,UnaryOperation) in …
fff6964 
…substitute_visitor.py
@rihi
Use visit instead of mapper function in visit_phi(self,Phi)
54dcae2
@rihi
Lift constraints of SubstituteVisitor that tried to uphold validity o…
f26128b 
…f the dataflow tree

Lift some constraints of what can be substituted. These constraints where in place in an attempt to prevent erroneous modifications to the dataflow tree, resulting in invalid states.
This commit shifts the responsibility of keeping the dataflow graph in a valid state to the user allowing for a more flexible implementation.
@rihi
Slight syntax changes to test_substitute_visitor.py
f3f012a
@rihi
Improve syntax in _OPERATION_TO_FOLD_FUNCTION
60dbc26
@rihi
Assert that instructions are not substituted in expression_simplifica…
b544b83 
…tion_rules.py
@rihi
Add clarifying comment to _simplify_instruction_with_rule
28cf1d1
@rihi
Rename ExpressionSimplificationRules to ExpressionSimplification
b4670cc
@rihi
Create copies while substituting with SubstituteVisitor.equality
3791dc1
@rihi rihi mentioned this pull request Sep 13, 2023
Closed
blattm
blattm reviewed Sep 18, 2023
View reviewed changes
decompiler/pipeline/controlflowanalysis/expression_simplification/rules/rule.py Show resolved Hide resolved
...mpiler/pipeline/controlflowanalysis/expression_simplification/rules/collapse_mult_neg_one.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification_rules.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/constant_folding.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/constant_folding.py Outdated Show resolved Hide resolved
.../pipeline/controlflowanalysis/expression_simplification/rules/simplify_trivial_arithmetic.py Show resolved Hide resolved
.../pipeline/controlflowanalysis/expression_simplification/rules/simplify_trivial_arithmetic.py Outdated Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/rules/term_order.py Show resolved Hide resolved
decompiler/pipeline/controlflowanalysis/expression_simplification/rules/sub_to_add.py Show resolved Hide resolved
...line/controlflowanalysis/expression_simplification/rules/test_simplify_trivial_arithmetic.py Show resolved Hide resolved
rihi and others added 19 commits September 20, 2023 15:40
@rihi @blattm
Update documentation for '_constant_fold_arithmetic_binary'
d2917b7 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi @blattm
Update documentation for 'normalize_int'
892c4fc 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi @blattm
Add comment explaining 'max_iterations'
1600341 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi @blattm
Improve documentation of CollectTerms
36215f0 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi @blattm
Rename variable in collect_terms.py for readability
8ddd6a1 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi @blattm
Rename variable in simplify_redundant_reference.py for readability
4f318ec 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi @blattm
Improve documentation of TermOrder
1c06528 
Co-authored-by: Manuel Blatt <[email protected]>
@rihi
Remove unused collapse_mult_neg_one.py
4efeebb
@rihi
Fix test in test_substitute_visitor.py
3146331
@rihi
Rename simplification rule FixAddSubSign to PositiveConstants
df201b9
@rihi
Rename variables in positive_constants.py
a1a7a1e
@rihi
Rename operands to constant in collect_terms.py
182913a
@rihi
Rename CollectTerms to CollapseNestedConstants
9207491
@rihi
Add missing unsigned multiply case to simplify_trivial_arithmetic.py
97ed807
@rihi
Use 'neg' operation in SubToAdd
b62a423
@rihi
Delete test_collapse_mult_neg_one.py
3679a36
@rihi
Fix test_stage.py
4a5392d
@rihi
Slight change to test_stage.py
61523ce
@blattm
Ignore MemPhi in SubstituteVisitor
5f22eb1
blattm
blattm approved these changes Sep 21, 2023
View reviewed changes
Copy link
Collaborator

@blattm blattm left a comment

Choose a reason for hiding this comment

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

Thank you!

@blattm blattm enabled auto-merge (squash) September 21, 2023 09:09
@blattm blattm merged commit a7d49d2 into main Sep 21, 2023
1 check passed
@blattm blattm deleted the issue-25-_Code_Generator_Simplify_Expressions branch September 21, 2023 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rihi rihi rihi left review comments

@blattm blattm blattm approved these changes

Assignees

@rihi rihi

Labels
feature-request New feature or request priority-low Low priority issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Code Generator] Simplify Expressions
2 participants
@rihi @blattm