Big update to prettier-extension-monkeyc

I have a question about code that might crash. I suppose this mostly affects untyped code...

Suppose I have something like:

class MyClass {
  const UniqueConstant = 42;
  var UniqueVariable = 5;
}

function foo(x) {
  if (x.UniqueVariable != 0) {
    return x.UniqueConstant;
  }
}

function bar(x) {
  return x.UniqueConstant;
}

function baz(x) {
  var y = x.UniqueConstant;
  return y;
}

The "Unique" names indicate that this is the only definition of a same-named entity anywhere in the program (ie there are no methods, enums, modules, classes or variables with those names anywhere else).

Now in foo, I see a use of x.UniqueVariable, and now I know for sure that x is an instance of MyClass. So now I can safely replace x.UniqueConstant by 42 (because if x is *not* an instance of MyClass, the program will crash and we won't get to the return). Nothing controversial so far - but this example is the motivation for doing this kind of type deduction.

In bar, I similarly know that *if* the function returns without crashing, it will return the value 42. But I don't want to remove the access to x.UniqueConstant, because now an incorrect program could continue to run, when it should have crashed. So I mark x.UniqueConstant as having a potential side effect, and the optimizer leaves it as it is. I think this is the right thing to do.

But now in baz, I try to do the same thing. I mark x.UniqueConstant as having side effects, so it's left as is. But the optimizer knows that y has the value 42. So the code ends up as:

function baz(x) {
  var y = x.UniqueConstant;
  return 42;
}

But now y is unused, so the code is converted to:

function baz(x) {
  x.UniqueConstant;
  return 42;
}

(note that it doesn't drop the x.UniqueConstant, because it was earlier marked as having potential side-effects). But now Garmin's compiler refuses to compile it, because x.UniqueConstant is an unused expression...

So do I just say wtf, the code is either correct, and returns the value 42, or it was going to crash, and I don't care what the optimized code does, so it might as well return 42...

Or do I give up on this kind of thing? I could also say that when I don't know the type of x for sure, I keep the type of x.UniqueConstant, but drop the value. In that case baz ends up mostly unchanged; but I'd still have the same problem with:

function buz(x) {
  var y = x.UniqueConstant;
  return y instanceof Lang.Number;
}

because the return statement gets optimized to "return true", so again y is dead, and we're left with a dangling initializer.

Also, note that these are pretty contrived; the main reason for doing this is that in a complex function with many uses of x, this approach lets us deduce the type of x from the first access (or first few accesses if the names aren't unique), and then we can optimize much better down stream. So the point is to allow the useful optimizations without falling into the traps listed above.

Am I just over thinking this? Should I just make a reasonable effort to preserve the "crashiness" of the original code, but not care about outliers like baz?

flocsy🤠 said:

The optimized code code is 2 bytes less, but when it runs then w2 is held in the memory

Note that there's a difference here between data types. As I understand it, Number and Float are 4 byte data types, with no (other) heap storage. ie a number on the stack, takes 4 bytes of stack, and no heap. Otoh, a Long and Double takes 4 bytes of stack, and 8 bytes of heap; and a string takes 4 bytes of stack and at least the string's length of heap memory.

The same applies to Array contents, for example. An array of size 50 already has space for 50 Numbers or 50 Floats, but if you put 50 Longs in it, you'll add 400 bytes to the heap. 

Also, as far as I can tell, the stack size doesn't contribute to your app's data footprint. So stack space is "free" (until you crash because you ran out of it, at any rate).

So yes, it's possible that PRE makes things worse when the values are Longs, Doubles or Strings. I should probably have an option controlling what should be allowed.

And regardless, I should manually re-use variables whose lifetimes don't overlap.

flocsy🤠 said:

And also I see a specific case when IMHO it could be easily optimized

If that's the exact code that didn't get optimized, I'm surprised; I see that being optimized pretty much everywhere. If there's a call between the two uses, I'm less surprised; although the optimizer does make an effort to analyze functions to determine which globals/member variables might be modified by any specific call (and that code got much better in the last release)

Actually, the other thing that sometimes breaks pre, is a call that possibly modifies the variable later in the code, followed by another use. ie

if (mResult != null) {
  extHR += " " + mResult;
}
...
foo();
return mResult;

If the optimizer can't prove that foo doesn't modify mResult, then in some cases it will see that as a group of three uses, which requires 2 assignments to the pre-variable, and so decides its not profitable (3 uses would be 24 bytes, but two assignments + 3 local uses is 26 bytes).

I have heuristics that attempt to break up the region so that it treats the first two as one set that *is* profitable, and the last use as a singleton that isn't profitable. But with the right control flow (and I don't think what I posted is quite right), it fails (again, something I'll come back to in the end - but solving this perfectly is a hard problem).

Nice blog. I figured out most of these tricks alone and with hard try. It could've save me some time if I find it earlier. There's one trick however that I don't understand. Why do you have this:

I think you are a little bit overthinking it. Add a sentence to the documentation that explains that the optimizer can work better if used with -l 3 and it is the recommended way to compile. Then make as much effort as you can to optimize in the best case (-l  3) scenario.

And if you still have time / need (though I bet that one good reason to make it work even better not with -l3 would be if your own projects don't compile with -l 3 and you want to be able to use it for them as well, BUT realizing the time and effort you put into the optimizer it's clearly not the point, 'cause then you could've fix all your projects to compile with strict mode in much less time :) then continue to try to improve it even without -l 3.

What I posted is the exact code and mResult is not used anywhere else after that in the function, and the function is not inlinable, however it is used "before" that in inlined code, so maybe this is the reason it didn't do the trick.

    (:memory16K)
    public function compute(info as Activity.Info) as Lang.Numeric or Time.Duration or Lang.String or Null {
        var extHr = computeExtHR();
        if (mResult2 != null) {
            extHr += " " + mResult2;
        }

        if (mTicker > FOUND_SECONDS) {
            extHr = strfHrZone(mConfFormat1, extHr, "", "", computeIntHR(info)); // inlined but doesn't have mResult2 in it
        }
        return extHr;
    }

    (:inline)
    hidden function computeExtHR() as AlphaNumeric {
        // ...
        var result2 = mResult2; // this was already manually optimized by me
        // ... result2 is used many times both as left and right value.
        mResult2 = result2;
    }

flocsy🤠 said:

# fix autobuild to a specific device

base.sourcePath=$(fr735xt.sourcePath)

Why not the "usual":

base.sourcePath = source

in eclipse the autobuild functioned without a specific device and the source folder alone does not build for me, I need a combo of source, source-ciq\some-version, source-device\some-device

in vscode it's probably no longer an issue as you always build for a specific device there.

markw65 said:

you probably don't want the optimizer to do it automatically

A good developer is lazy, if it can be automatic, I prefer automation.

Another challenge is probably that you would have to recurse through the tree as there can be multiple inheritance levels.

Anyway currently I'm not a user of your extension, but if you get to implementing class inheritance elimination, you can ping me and I'll test it out :) 

(:no_api2, :inline, :typecheck(false))
function getConfig(key as PropertyKeyType) as PropertyValueType {
    try {
        return ExtHRMApp.getProperty(key);
    } catch (e) {
        return null;
    }
}

After adding the try/catch I get the following error during optimization:

Starting optimization step...
ERROR: Internal: Error: Got exception `ReturnStatement got lost!' while processing node $.getConfigBool:VariableDeclaration from source/Config.mc:72:72
Error: Got exception `ReturnStatement got lost!' while processing node $.getConfigBool:VariableDeclaration from source/Config.mc:72:72
at inlineWithArgs (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:5691:19)
at inlineFunctionHelper (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:5782:16)
at inlineFunction (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:5792:17)
at optimizeCall (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:9160:25)
at state.post (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:8904:53)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:6365:40
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:437:20)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:398:34
at Array.reduce (<anonymous>)
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:396:38)
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:417:24)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:398:34
at Array.reduce (<anonymous>)
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:396:38)
at Object.traverse (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:6062:20)
at api_collectNamespaces (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:6437:11)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:8996:63
at Array.forEach (<anonymous>)
at optimizeMonkeyC (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:8995:26)
at async Promise.all (index 1)
at async generateOptimizedProject (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:9569:5)
at async buildOptimizedProject (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:9267:61)

* The terminal process terminated with exit code: 1.
* Terminal will be reused by tasks, press any key to close it.

* Executing task: omonkeyc: epix

I understand that it can't be inlined because there are 2 returns, but util recently in this case there was a warning.

flocsy🤠 said:

After adding the try/catch I get the following error during optimization

This one repros for me. I should be able to fix it.

flocsy🤠 said:

What I posted is the exact code and mResult is not used anywhere else after that in the function, and the function is not inlinable, however it is used "before" that in inlined code, so maybe this is the reason it didn't do the trick

I'm sure thats it. When I said "later" I should have just said elsewhere in the function. And obviously, it its there because of inlining thats all the same (pre runs after all the other optimizations). But I've tried to repro, by doing something along the lines you presented, and I've had no luck. Again, if there's any chance you can cut it down to something that exhibits the problem, and that you're willing to share, that would really help.