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

Whether to reduce or not, elaborated vs. non-elaborated #3759

Open
hs-apotell opened this issue Jul 16, 2023 · 2 comments
Open

Whether to reduce or not, elaborated vs. non-elaborated #3759

hs-apotell opened this issue Jul 16, 2023 · 2 comments

Comments

@hs-apotell
Copy link
Collaborator

Usually, the check for whether or not to reduce an expression is dependent on the reduce argument to the CompileHelper members. However, the flag doesn't always account for whether the functions are being called in elaborated mode or non-elaborated mode context. There are cases where the functions explicitly force a reduction by passing true downstream disregarding the input reduce flag. I have tried to fix this issue in currently open/blocked PR #3651, but ran into other unexpected differences.

Question: Is wrapping the reduction call sites with both both m_elabMode and reduce the right solution to disable reduction in non-elaborated mode? If not, what more need to happen?

P.S. I pointed out this issue in the recently merged PR #3758 as well but since it didn't catch your eye, reporting it as an actual issue since this needs to be accounted for in lot more other places than just this specific change.

@alaindargelas
Copy link
Collaborator

I think we are at a state where we should not force reduction in any functions. But there might still be issues with constant propagation that needs fixing if we do so.
Forcing reduction was usually a short-term fix for some constant pushing not working otherwise.

@hs-apotell
Copy link
Collaborator Author

I will introduce the additional check and add a verification to make sure the elaborated tree doesn't change with my edits. We can address the specific cases where doing so modified the elaborated tree.

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

No branches or pull requests

2 participants