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

Enum inlining no longer working #11332

Open
ncannasse opened this issue Oct 16, 2023 · 9 comments
Open

Enum inlining no longer working #11332

ncannasse opened this issue Oct 16, 2023 · 9 comments

Comments

@ncannasse
Copy link
Member

There used to be an optimization regarding enum inline, so the following would not trigger any allocation.

enum E {
	X( v : Int );
	Y( s : String );
	Z;
}

class Main {

	static inline function foo(x:E) : Dynamic {
		return switch( x ) {
		case X(v): v * 2;
		case Y(s): s+"!";
		case Z: "hello";
		}
	}

	static function main() {
		trace(foo(X(33)));
		trace(foo(Y("x"+"z")));
		trace(foo(Z));
	}

}

This should entirely remove the switch in inline code as well as allocation of enum E.X/Y.

@Simn
Copy link
Member

Simn commented Oct 16, 2023

I'm getting this with -D analyzer-optimize:

Main.main = function() {
	haxe_Log.trace(66,{ fileName : "source/Main.hx", lineNumber : 17, className : "Main", methodName : "main"});
	var x = E.Y("x" + "z");
	haxe_Log.trace(x.s + "!",{ fileName : "source/Main.hx", lineNumber : 18, className : "Main", methodName : "main"});
	haxe_Log.trace("hello",{ fileName : "source/Main.hx", lineNumber : 19, className : "Main", methodName : "main"});
};

@Simn
Copy link
Member

Simn commented Nov 9, 2023

I checked various Haxe versions in try.haxe and all of them have similar output without -D analyzer-optimize, so I don't think there's a regression here.

@Simn Simn closed this as completed Nov 9, 2023
@ncannasse ncannasse reopened this Dec 6, 2024
@ncannasse
Copy link
Member Author

@Simn not a regression but could be nice wrt inlining optims

@basro
Copy link
Contributor

basro commented Dec 6, 2024

This is inlining properly in the js target:

// Generated by Haxe 4.3.6
(function ($global) { "use strict";
class Main {
	static main() {
		console.log("Main.hx:17:",66);
		console.log("Main.hx:18:","x" + "z" + "!");
		console.log("Main.hx:19:","hello");
	}
}
class haxe_iterators_ArrayIterator {
	constructor(array) {
		this.current = 0;
		this.array = array;
	}
	hasNext() {
		return this.current < this.array.length;
	}
	next() {
		return this.array[this.current++];
	}
}
{
}
Main.main();
})({});

@Speedphoenix
Copy link

Speedphoenix commented Dec 9, 2024

I tried with this code, and the js output on try haxe indeed inlines everything into a single trace https://try.haxe.org/#5dE60066

enum E {
	X(v:Int);
	Y(s:String);
	Z;
}

class Test {
	static function main() {
		inline function fromE(v:E) {
			return switch (v) {
				case X(i): i;
				case Y(str): str.length;
				case Z: -1;
			}
		}
		var a1 = X(5);
		var b1 = Y("hello2");
		var c = Z;

		var gotten1 = fromE(a1);
		var gotten3 = fromE(b1);
		var gotten5 = fromE(c);
		trace(gotten1, gotten3, gotten5);
	}
}

Js output (Generated by Haxe 4.3.6):

haxe_Log.trace(5,{ fileName : "Test.hx", lineNumber : 23, className : "Test", methodName : "main", customParams : ["hello2".length,-1]});

Meanwhile, the -D dump=pretty output on target hl allocates enum instances

var a1 = E.X(5);
var b1 = E.Y("hello2");
var c = E.Z;
var gotten1;
switch ((enumIndex a1)) {
	case 0: {
		var i = a1[0];
		gotten1 = i;
	};
	case 1: {
		var str = a1[0];
		gotten1 = str.length;
	};
	case 2: gotten1 = -1;
};
var gotten3;
switch ((enumIndex b1)) {
	case 0: {
		var i = b1[0];
		gotten3 = i;
	};
	case 1: {
		var str = b1[0];
		gotten3 = str.length;
	};
	case 2: gotten3 = -1;
};
var gotten5;
switch ((enumIndex c)) {
	case 0: {
		var i = c[0];
		gotten5 = i;
	};
	case 1: {
		var str = c[0];
		gotten5 = str.length;
	};
	case 2: gotten5 = -1;
};
haxe.Log.trace(gotten1, {fileName : "src/Main.hx", lineNumber : 25, className : "Main", methodName : "main", customParams : [gotten3, gotten5]});

@Speedphoenix
Copy link

Speedphoenix commented Dec 9, 2024

Using my local haxe.exe the js output balloons too, so this might actually be a regression.
I am currently on haxe 7f3dd53

var a1 = prefab.E.X(5);
var b1 = prefab.E.Y("hello2");
var c = prefab.E.Z;
var gotten1;
switch(a1._hx_index) {
case 0:
	var i = a1.v;
	gotten1 = i;
	break;
case 1:
	var str = a1.s;
	gotten1 = str.length;
	break;
case 2:
	gotten1 = -1;
	break;
}
var gotten3;
switch(b1._hx_index) {
case 0:
	var i = b1.v;
	gotten3 = i;
	break;
case 1:
	var str = b1.s;
	gotten3 = str.length;
	break;
case 2:
	gotten3 = -1;
	break;
}
var gotten5;
switch(c._hx_index) {
case 0:
	var i = c.v;
	gotten5 = i;
	break;
case 1:
	var str = c.s;
	gotten5 = str.length;
	break;
case 2:
	gotten5 = -1;
	break;
}
haxe.Log.trace(gotten1,{ fileName : "src/prefab/FXPlayer.hx", lineNumber : 46, className : "prefab.FXPlayer", methodName : "makeObject", customParams : [gotten3,gotten5]});

@kLabz
Copy link
Contributor

kLabz commented Dec 9, 2024

Why would it be a regression? try-haxe does inline properly on latest nightly (while the one you're using is older).

Are you using -D analyzer-optimize?

@Speedphoenix
Copy link

I was indeed not using -D analyzer-optimize, and adding it yields the same result as on try-haxe

@Simn
Copy link
Member

Simn commented Dec 9, 2024

I think the only thing to potentially change here is making -D analyzer-user-var-fusion the default. IIRC we don't always do it because it can be quite annoying for debugging, but opting out of it seems more sensible than opting in.

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

5 participants