Nested variable declarations in for loop difference (of CPU vs GPU) about gpu.js HOT 9 CLOSED

Thanks for checking out the demo! And also for pointing out the bug. The change in lighting is actually a bug in my code, it was looping for lights using i and somewhere below I redeclared the i. Fixed it in my latest commit. Think it works when in GPU mode probably because the generated code takes into account the scope for i?

from gpu.js.

@staceytay : ideally, both CPU and GPU mode should be consistent. And if it isn't it should throw a warning / exception.

For clarification, is the following what you meant?

for(var i=0; i<max; ++i) {
    for(var i=0; i<max; ++i) {
        //do something
    }
    //do something else
}

And somehow the 2nd inner i instead of being a different scope (as per JS) is actually now the same scope (as per GPU.JS)?

Which i am guessing happens cause we unroll out the iteration variable declarations to outside the loop.

Either way, if redeclaration of vars is creating unexpected behaviour, I should throw a warning / exception on that.

from gpu.js.

Yup that's what I meant for the for loop.

That's true, the two modes should be consistent. On a related note, there's also an issue with using this.constants in kernel functions in CPU mode. In CPU mode you'll get a this.constants is undefined error although it works in GPU mode.

from gpu.js.

@PicoCreator You've got it upside down. JavaScript does not have block scoping, it has functional scoping. Its GLSL that has block scoping. We must follow JavaScript semantics so ideally i should be same scope instead of different scope on the gpu (crazy, I know).

I suppose we could either implement variable hoisting, or treat this as undefined behaviour. Personally don't see any sane use for variable hoisting and I wish I could rather just emit a big error about undefined behaviour if hoisting is used anywhere in the kernel. I don't mind if anybody wants to implement it though.

from gpu.js.

Hey Stace! Good to see you here haha. Sorry for hijacking the issue, just wanted to say hi :). Sadly I'm the languages guy. Haven't touched the code in ages, but I will after exams so yeaaa. We were talking about using a precompiler for CPU mode to enforce block scope right? Implementing scope either way is easy for GPU mode, it's just another layer in the environment. Personally I think keeping it JS and cui is better, as long as people are smart about their scoping.

from gpu.js.

Hi Matthew! Yea I think keeping it JS, and emitting a warning or putting this somewhere in the documentation is fine. It's more about the user expecting the code to be the same in both CPU and GPU mode.

from gpu.js.

Merged with #35

from gpu.js.

@fuzzie360 : yup your right had it backwards, lol. Realised it when i did up the issue 31 test case. bad8f04

@sawman : My current suggestion would be to actually flag out a warning / exception, as we generate the code from AST. On repeated variable declaration (repeated var declares are a bad idea anyway)

This would be an extension of the addFunction script, which already keeps track of function declarations and calling trace (automatically skipping functions that do not get called).

Sorry been too busy the last week running around Singapore for work to check things up.

from gpu.js.

Handled here: robertleeplummerjr@b999dc6#diff-d02bb56a83c0737e60796c843f0857c9R461
From: #66

from gpu.js.