Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Rutefig/231 applying cse to compiler #264

Merged
merged 57 commits into from
Aug 22, 2024

Conversation

rutefig
Copy link
Contributor

@rutefig rutefig commented Jul 2, 2024

No description provided.

@rutefig rutefig requested review from leolara and alxkzmn July 2, 2024 16:28
@rutefig rutefig self-assigned this Jul 2, 2024
src/poly/cse.rs Outdated Show resolved Hide resolved
src/poly/cse.rs Outdated Show resolved Hide resolved
@rutefig rutefig requested a review from leolara August 8, 2024 18:45
@leolara
Copy link
Collaborator

leolara commented Aug 9, 2024

Why is not passing the tests?

@leolara
Copy link
Collaborator

leolara commented Aug 9, 2024

We need a trace that have a method, that received the relevant info about the common sub-expression and returns a u32. 0 means that it should be ignored and other number is the score and the biggest score should be picked to subsitute.

@rutefig sorry, there where it says trace it should have said trait. Please, let me know if you don't understand something.

An object that implements a trait should be passed to return the score.

When that is done, let me know, to review again

@rutefig
Copy link
Contributor Author

rutefig commented Aug 9, 2024

Why is not passing the tests?

Just formatting, I will fix it when it would be ready for review

@rutefig
Copy link
Contributor Author

rutefig commented Aug 10, 2024

We need a trace that have a method, that received the relevant info about the common sub-expression and returns a u32. 0 means that it should be ignored and other number is the score and the biggest score should be picked to subsitute.

@rutefig sorry, there where it says trace it should have said trait. Please, let me know if you don't understand something.

An object that implements a trait should be passed to return the score.

When that is done, let me know, to review again

@leolara Can you review again pls?

src/compiler/cse.rs Outdated Show resolved Hide resolved
@leolara
Copy link
Collaborator

leolara commented Aug 13, 2024

@rutefig we need a default implementation of the scorer following this rules as explained before:

The first implementation should be:

if degree is less than a configurable number return 0
if number of repetations is less than a configurable number return 0
return number of occurances * degree

@leolara
Copy link
Collaborator

leolara commented Aug 13, 2024

The scorer is good, now I need to review the rest. I have not checked the tests, but make sure they are comprehensive and check corner cases.

@rutefig rutefig requested a review from leolara August 15, 2024 09:01
src/compiler/cse.rs Outdated Show resolved Hide resolved
src/compiler/cse.rs Outdated Show resolved Hide resolved
src/compiler/cse.rs Show resolved Hide resolved
src/compiler/cse.rs Show resolved Hide resolved
src/compiler/cse.rs Outdated Show resolved Hide resolved
src/compiler/cse.rs Outdated Show resolved Hide resolved
src/compiler/cse.rs Outdated Show resolved Hide resolved
src/compiler/cse.rs Show resolved Hide resolved
src/poly/cse.rs Outdated Show resolved Hide resolved
}

impl<F, V> Default for ConstrDecomp<F, V> {
impl<F, V: Debug, M> ConstrDecomp<F, V, M> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean what is the method get_auto_signal for?

src/sbpir/mod.rs Outdated Show resolved Hide resolved
@rutefig rutefig requested a review from leolara August 19, 2024 10:33
src/poly/cse.rs Outdated Show resolved Hide resolved
@rutefig rutefig merged commit 9c08b6d into chiquito-2024 Aug 22, 2024
4 checks passed
@rutefig rutefig deleted the rutefig/231-applying-cse-to-compiler branch August 22, 2024 14:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants