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

flattenLet applied in bottomup traversal #2599

Merged
merged 2 commits into from
Nov 7, 2023
Merged

flattenLet applied in bottomup traversal #2599

merged 2 commits into from
Nov 7, 2023

Conversation

christiaanb
Copy link
Member

Ensures deeply nested let-bindings are flattened first so that let-expressions do not get inlined into argument positions in their unflattened form.

Has some minor performance effects.

Before:

benchmarking normalization of examples/Reducer.hs
time                 395.2 ms   (385.3 ms .. 410.2 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 401.3 ms   (396.8 ms .. 406.4 ms)
std dev              5.429 ms   (1.784 ms .. 7.434 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking normalization of tests/shouldwork/Basic/AES.hs
time                 161.3 ms   (160.1 ms .. 162.5 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 164.9 ms   (163.1 ms .. 169.4 ms)
std dev              4.042 ms   (490.2 μs .. 5.968 ms)
variance introduced by outliers: 12% (moderately inflated)

After:

benchmarking normalization of examples/Reducer.hs
time                 447.0 ms   (439.0 ms .. 451.0 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 454.4 ms   (451.0 ms .. 460.9 ms)
std dev              6.411 ms   (13.17 μs .. 7.496 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking normalization of tests/shouldwork/Basic/AES.hs
time                 182.2 ms   (180.8 ms .. 183.4 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 185.3 ms   (183.9 ms .. 188.1 ms)
std dev              2.666 ms   (490.4 μs .. 3.715 ms)
variance introduced by outliers: 14% (moderately inflated)

Fixes #2598

@christiaanb christiaanb force-pushed the fix2598 branch 2 times, most recently from 2b5af0d to 749c55a Compare November 6, 2023 15:08
@DigitalBrains1
Copy link
Member

About 13% slower isn't really a minor effect though?

Ensures deeply nested let-bindings are flattened first so that
let-expressions do not get inlined into argument positions in
their unflattened form.

Has some minor performance effects.

Before:
```
benchmarking normalization of examples/Reducer.hs
time                 395.2 ms   (385.3 ms .. 410.2 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 401.3 ms   (396.8 ms .. 406.4 ms)
std dev              5.429 ms   (1.784 ms .. 7.434 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking normalization of tests/shouldwork/Basic/AES.hs
time                 161.3 ms   (160.1 ms .. 162.5 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 164.9 ms   (163.1 ms .. 169.4 ms)
std dev              4.042 ms   (490.2 μs .. 5.968 ms)
variance introduced by outliers: 12% (moderately inflated)
```

After:
```
benchmarking normalization of examples/Reducer.hs
time                 447.0 ms   (439.0 ms .. 451.0 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 454.4 ms   (451.0 ms .. 460.9 ms)
std dev              6.411 ms   (13.17 μs .. 7.496 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking normalization of tests/shouldwork/Basic/AES.hs
time                 182.2 ms   (180.8 ms .. 183.4 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 185.3 ms   (183.9 ms .. 188.1 ms)
std dev              2.666 ms   (490.4 μs .. 3.715 ms)
variance introduced by outliers: 14% (moderately inflated)
```

Fixes #2598
@christiaanb
Copy link
Member Author

Right, but the other benchmarks do not increase.

@christiaanb christiaanb merged commit 1c373ef into master Nov 7, 2023
@christiaanb christiaanb deleted the fix2598 branch November 7, 2023 13:25
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.

Name duplication in generated Verilog for ClockWizard testbench
2 participants