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

Redundant function input args slicing #47

Open
dy opened this issue Dec 16, 2018 · 5 comments
Open

Redundant function input args slicing #47

dy opened this issue Dec 16, 2018 · 5 comments

Comments

@dy
Copy link
Member

dy commented Dec 16, 2018

Transpiling

vec4 lerp(vec4 a, vec4 b, float t) {
	return t * b + (1. - t) * a;
}

gives

		function lerp (a, b, t) {
			a = a.slice();
			b = b.slice();
			return [t * b[0] + (1. - t) * a[0], t * b[1] + (1. - t) * a[1], t * b[2] + (1. - t) * a[2], t * b[3] + (1. - t) * a[3]];
		}

which has redundant slicing: slower performance, longer output. Probably we should guard mutation only in case of out qualifiers.

@Pessimistress what do you think?

@Pessimistress
Copy link
Contributor

Consider this function:

bool isVisible(vec4 position_clipspace) {
  position_clipspace.z /= position_clipspace.w;
  return position_clipspace.z > -1.0;
}

Transpiles to

function isVisible (position_clipspace) {
  position_clipspace[2] /= position_clipspace[3];
  return position_clipspace[2] > -1;
};

The input variable should not be mutated according to https://www.khronos.org/opengl/wiki/Core_Language_(GLSL)#Parameters

I acknowledge that this is likely very rare and not good coding practice anyways. I agree the slicing feels like too much overhead than it's worth. Is there anyway to detect mutation and only slice the input in that case?

@dy
Copy link
Member Author

dy commented Dec 16, 2018

We would have to introduce "horizontal knowledge" for nodes, like constant propagation etc.

Right now we have only "vertical" knowledge: children, parent nodes.

Alternatively, we could try to detect if a function modifies its arguments, that should not be very difficult - just iterate over all nested nodes, check if an argument is engaged in assignment nodes.

Mb we could also figure that out somehow from the scope stats - track variables state.

@dy
Copy link
Member Author

dy commented Dec 16, 2018

Seems that the project may need refactoring soon, if we find a sponsor. Tech debt accumulates, the code oftentimes vague and unclear, like variables tracking, descriptor init cases etc, needs more purity.

@Pessimistress
Copy link
Contributor

I'm ok with removing the argument slicing for now and maybe adding a note in the readme - we do not have any real world use case that is affected by this.

@dy
Copy link
Member Author

dy commented Dec 17, 2018

@Pessimistress that's fine, let's keep it. We have many other redundancies.

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