Filed PR jit/66812 for the code generation issue

[David Malcolm]

On Thu, 2015-07-09 at 01:12 +0100, Dibyendu Majumdar wrote:
> On 8 July 2015 at 21:06, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> >> BTW, are Lua/Ravi constants truly constant?  If so, then I'd believe
> >> you'd get a performance win by implementing LOADK by emitting code to
> >> write the specific tt and value_ directly, rather than code that copies
> >> a value from the table.
> >
> > Yes - I do specialize to constants in certain cases - such as loops
> > and numeric operations. But more can be done.
> >
> >>
> >> This would enable the optimizer to "know" the tt and value_, and
> >> optimize accordingly.  For example, in this case, I believe it would
> >> allow the function to be optimized away down to the equivalent of just a
> >> "return false;".  Obviously won't help much for a function without a
> >> loop, but if it saves instructions inside a loop, that's probably a win.
> >>
> >> (...though maybe not before we track down this issue)
> >>
> 
> I implemented setting the constant directly (just checked in).
> Performance wise no noticeable difference (as I think the scenarios
> where this helps were already covered)

Ah, ok.

> but it does seem to have caused
> the optimization bug to go away - i.e. the tests now pass. I will run
> more tests tomorrow to see if this is true under all optimization
> settings.

FWIW, I suspect that this is effectively just papering over the problem,
and that it will come back to bite us, just with a more complicated
reproducer :(

Does something like:



function x(some_arg)
  local IX
  if ((some_arg or true) and false) then
    IX = true
  end;
  return ((some_arg or true) and false)
end
assert(x() == false)



exhibit the bug?  (have never programmed Lua, so this probably even
isn't syntactically correct, but hopefully you get the idea, with
const-propagation of 10's value and typecode lots of code gets optimized
away, so using an arg of unknown type to thwart that and hopefully get
the bug to re-manifest itself).


(Note that I've managed to isolate a minimal reproducer for the problem,
as noted in another mail this thread)