Skip to content

Conversation

shakesoda
Copy link

@shakesoda shakesoda commented Dec 17, 2022

This is pretty gross to do, so I don't know if you'll want to merge it, but the codegen for ternaries on Lua target is truly terrible and I thought I'd bring it up. I think this is also the only set of functions where switching it to a Lua builtin would be relevant, everything else would just need a plain if instead of ternary.

This also affects every other usage of ternaries such as in sign, step, normalize, faceforward and reflect, I just picked on min/max first because there's a standard function for it. I don't think other targets have this codegen problem, but spitting out this many closures constantly does have gc/perf consequences on Lua.

Before:

local min = Vec3Data.new((function() 
  local _hx_1
  if (b.x < p.x) then 
  _hx_1 = b.x; else 
  _hx_1 = p.x; end
  return _hx_1
end )(), (function() 
  local _hx_2
  if (b.y < p.y) then 
  _hx_2 = b.y; else 
  _hx_2 = p.y; end
  return _hx_2
end )(), (function() 
  local _hx_3
  if (b.z < p.z) then 
  _hx_3 = b.z; else 
  _hx_3 = p.z; end
  return _hx_3
end )());

After:

local min = Vec3Data.new(_G.math.min(b.x, p.x), _G.math.min(b.y, p.y), _G.math.min(b.z, p.z));

Alternatively without the special casing:

public extern overload inline function min(b: Vec3): Vec3 {
	var mx = x;
	var my = y;
	var mz = z;
	if (b.x < x) { mx = b.x; }
	if (b.y < y) { my = b.y; }
	if (b.z < z) { mz = b.z; }
	return new Vec3(mx, my, mz);
}

outputs:

local mx = p.x;
local my = p.y;
local mz = p.z;
if (b.x < p.x) then 
  mx = b.x;
end;
if (b.y < p.y) then 
  my = b.y;
end;
if (b.z < p.z) then 
  mz = b.z;
end;
local min = Vec3Data.new(mx, my, mz);

(all results from haxe 4.2.5 using -D lua-vanilla -D analyzer-optimize -dce full

@skial skial mentioned this pull request Jan 4, 2023
1 task
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.

1 participant