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

Save to temporary when side effects may modify the first operand #2352

Merged
merged 2 commits into from
May 14, 2020

Conversation

mihaibudiu
Copy link
Contributor

No description provided.

Copy link
Contributor

@ChrisDodd ChrisDodd left a comment

Choose a reason for hiding this comment

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

This sort of thing is why having the spec define order of operations (instead of leaving it as undefined behavior) is IMO a mistake.

@mihaibudiu mihaibudiu merged commit e476562 into p4lang:master May 14, 2020
@mihaibudiu mihaibudiu deleted the issue2205 branch May 14, 2020 20:09
@jafingerhut
Copy link
Contributor

@jnfoster ChrisDodd @mbudiu-vmw

In my understanding, it seems that the two approaches being discussed here are:

(a) make the language spec such that for nearly arbitrarily complex "side-effecty" code that is syntactically correct, there is one correct behavior for an implementation to perform. That seems to be the path that P4_16 has chosen most often for things like this.

(b) make the language spec such that when the order of evaluation of things that 'look symmetric' in the language is important, the implementation is free to do whatever it wants, e.g. it can implement expr1+expr2 or f(expr1, expr2), evaluating expr1 and expr2 in any order it wishes, even if that could produce different results.

Are there any programming languages that you know of that take an approach like the following?

(c) Whenever an expression whose result depends upon the order of evaluation is encountered, (e.g. expr1 + expr2 or f(expr1, expr2), where evaluation of expr1 and expr2 have data dependencies between them), the compiler points out this data dependency in some way, and treats it as an error. The developer is responsible for changing the code to make the order of evaluation more explicit, e.g. tmp1 = expr1; f(tmp1, expr2);.

Why do I ask? Because such an approach avoids undefined behavior (or multiple possible legal behaviors from different implementations), and it also forces developers to write code whose behavior can be understood by themselves and other developers, without needing to remember all of the subtleties of a language spec's order of evaluation. I also suspect that such code is not written terribly often by typical P4 developers, but do not currently have any actual data to back that up. I suspect some of them would actually thank you for disallowing them to write code in that way.

@mihaibudiu
Copy link
Contributor Author

We already have an evaluation order in the spec, so I don't think we can go back without breaking user programs. What you ask could be part of a linter.

@jafingerhut
Copy link
Contributor

@ChrisDodd: "This sort of thing is why having the spec define order of operations (instead of leaving it as undefined behavior) is IMO a mistake."

Would you mind elaborating slightly why you consider it a mistake?

I can imagine a couple of possible reasons:

(a) You expect that the language specification being written that way will encourage P4 developers to write code that relies upon the specified evaluation order, leading to data dependencies that make their compiled program more expensive on several kinds of useful targets.

(b) You expect that non-optimally written front/mid-end passes of a P4 compiler will introduce extra assignments even when there are not any actual data dependencies, and you expect such additional assignments make it difficult for a back end to generate parallel code in such cases.

But perhaps there are other reasons I have not imagined.

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