Re: Filed PR jit/66812 for the code generation issue

[Dibyendu Majumdar <mobile@majumdar.org.uk>]

On 9 July 2015 at 22:06, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2015-07-09 at 21:52 +0100, Dibyendu Majumdar wrote:
>> On 9 July 2015 at 20:22, David Malcolm <dmalcolm@redhat.com> wrote:
>> >> 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 :(
>> >
>>
>> Yes agree.
>>
>> > 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).
>> >
>>
>> Actually that is valid Lua. Since you aren't passing a value to x()
>> then some_arg will be NIL. In Lua, NIL and false are false, everything
>> else is true.
>> The example above works okay as is, and also when I pass 10 to x().
>> As usual with certain bugs - only a specific set of circumstances trigger it.
>>
>> >
>> > (Note that I've managed to isolate a minimal reproducer for the problem,
>> > as noted in another mail this thread)
>> >
>>
>> Yes that is very good news. Hope you will soon discover the root cause.
>
> Root cause found: it turns out that part of GCC's alias analysis is
> deferred to a frontend-specific hook:
>   LANG_HOOKS_GET_ALIAS_SET

Fanstastic news!

>
> For C/C++/Objective C this hook allows type-punning when directly
> accessing unions, and the Ravi code assumes this.
>
> I hadn't implemented this hook for libgccjit, so the optimizer assumes
> that in this sequence of assignments:
>
>   (1) R1.value_.b = 0;
>   (2) R1.value_.i = CONST0.value_.i;
>   (3) R1.value_.b = 0;
>
> that (1) and (3) have the same LHS, but (2) has a different LHS.
>
> Hence it assumes that (3) is redundant, due to (1), and optimizes it
> away.
>
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66812 for more gory
> details.
>
> The fix will be to implement the LANG_HOOKS_GET_ALIAS_SET internal GCC
> API thus giving libgccjit some rules about aliasing.  Some options:
>
> (i) make it identical to C.
> (ii) give the client code some control over this
>
> My initial gut feeling is to go with (i).

One of the pains in LLVM is that one has to manually provide aliasing
info - so while I don't fully understand what above means - if the
client code gets C like behaviour by default that would be great.
Maybe later on you can provide more flexibility.

Regards
Dibyendu