[Bug jit/103562] New: Jitted code produces incorrect result when returning 3-member struct from internal function

[Bug jit/103562] New: Jitted code produces incorrect result when returning 3-member struct from internal function

[andy.pj.hanson at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

            Bug ID: 103562
           Summary: Jitted code produces incorrect result when returning
                    3-member struct from internal function
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: jit
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: andy.pj.hanson at gmail dot com
  Target Milestone: ---

Created attachment 51927
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51927&action=edit
File that reproduces the bug

See the attached program `jit_error.c`. To reproduce, run: `gcc jit_error.c
-lgccjit && ./a.out`. This will output: `get_a(&s) is 140730936729392` (or some
other large number); The correct output would be 1.

The file should JIT a program like this:
```
struct my_struct { long a; long b; long c; };
struct my_struct deref(struct my_struct *ptr) { return *ptr; } 
long get_a(struct my_struct *s) { return deref(s).a; }
```

For some reason, the function `deref` is optimized in an invalid way.

The bug goes away (and the program correctly outputs 1) if you do any one of
the following:

* Set GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL to 0.
* Remove the `c` field.
* Change `func_deref` from `GCC_JIT_FUNCTION_INTERNAL` to
`GCC_JIT_FUNCTION_EXPORTED`.
* Inline the call using `gcc_jit_rvalue *callDeref =
gcc_jit_lvalue_as_rvalue(gcc_jit_rvalue_dereference(gcc_jit_param_as_rvalue(param_get_a),
NULL));`.
* Use `GCC_JIT_TYPE_INT` instead of `GCC_JIT_TYPE_LONG`.

I've tested this the GCC master branch as of 20211204, and with OpenSUSE's GCC
version.

My `gcc -v` is:

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/home/andy/temp/gccjitrepro2/install/libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../src/configure --enable-host-shared
--enable-languages=jit,c++ --disable-bootstrap --disable-shared
--prefix=/home/andy/temp/gccjitrepro2/install
--with-gmp=/home/andy/temp/gccjitrepro2/build-gmp
--with-isl==/home/andy/temp/gccjitrepro2/build-isl
--with-mpfr=/home/andy/temp/gccjitrepro2/build-mpfr
--with-mpc=/home/andy/temp/gccjitrepro2/build-mpc
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.0.0 20211204 (experimental) (GCC)

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-12-06
                 CC|                            |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Thanks for the report. I added:

  gcc_jit_context_set_bool_option(ctxt, GCC_JIT_BOOL_OPTION_DUMP_EVERYTHING,
1);
  gcc_jit_context_set_bool_option(ctxt, GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES,
1);

and I can see in the very first dump file:

$ cat fake.c.006t.gimple
struct my_struct deref (struct my_struct * ptr)
{
  <D.79>:
  <retval> = *ptr;
  return <retval>;
}


long int get_a (struct my_struct * ptr)
{
  long int D.87;
  struct my_struct D.88;

  <D.83>:
  D.88 = deref (ptr); [return slot optimization]
  D.87 = D.88.a;
  return D.87;
}

So there's fishy the [return slot optimization].
@David: Can you please take a look?

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #1)
> So there's fishy the [return slot optimization].
RSO should be fine there, that is what you get even with the C code version
from GCC (-O2 -fno-inline):

;; Function deref (deref, funcdef_no=0, decl_uid=1982, cgraph_uid=1,
symbol_order=0)

struct my_struct deref (struct my_struct * ptr)
{
  <bb 2> [local count: 1073741824]:
  <retval> = *ptr_2(D);
  return <retval>;

}

;; Function get_a (get_a, funcdef_no=1, decl_uid=1985, cgraph_uid=2,
symbol_order=1)

long int get_a (struct my_struct * s)
{
  struct my_struct D.1993;
  long int _4;

  <bb 2> [local count: 1073741824]:
  D.1993 = deref (s_2(D)); [return slot optimization]
  _4 = D.1993.a;
  return _4;

}

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org
           Priority|P3                          |P1

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
It's very fishy and it's screwed up in the inliner:

$ cat pr103562-2.c
struct my_struct { long a; long b; long c; };
static struct my_struct deref(struct my_struct *ptr) { return *ptr; } 
long get_a(struct my_struct *s) { return deref(s).a; }

...

$ gcc pr103562-2.c -O1 -c -fdump-tree-all -fdump-ipa-all-details

pr103562-2.c.082i.fnsummary contains:

long int get_a (struct my_struct * s)
{
  struct my_struct D.1989;
  long int _4;

  <bb 2> [local count: 1073741824]:
  D.1989 = deref (s_2(D)); [return slot optimization]
  _4 = D.1989.a;
  return _4;

}


struct my_struct deref (struct my_struct * ptr)
{
  <bb 2> [local count: 1073741824]:
  <retval> = *ptr_2(D);
  return <retval>;

}

pr103562-2.c.083i.inline does:

long int get_a (struct my_struct * s)
{
  struct my_struct D.1989;
  long int _4;

  <bb 2> [local count: 1073741824]:
  D.1989 = *s_2(D);
  _4 = D.1989.a;
  return _4;

}

While using JIT with the following patch:

diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index b412eae6aa8..ccde56cdd98 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -2512,9 +2512,9 @@ make_fake_args (vec <char *> *argvec,

   if (get_bool_option (GCC_JIT_BOOL_OPTION_DUMP_EVERYTHING))
     {
-      ADD_ARG ("-fdump-tree-all");
-      ADD_ARG ("-fdump-rtl-all");
-      ADD_ARG ("-fdump-ipa-all");
+      ADD_ARG ("-fdump-tree-all-details");
+      ADD_ARG ("-fdump-rtl-all-details");
+      ADD_ARG ("-fdump-ipa-all-details");
     }

   /* Add "-fdump-" options for any calls to

$ gcc pr103562.c -lgccjit && ./a.out
using libgccjit 12.0.0
intermediate files written to /tmp/libgccjit-kH6M9y
get_a(&s) is 140737488346368

$ cd /tmp/libgccjit-kH6M9y

fake.c.083i.inline does:

Processing frequency deref/0
  Called by get_a/1 that is normal or hot
                Accounting size:-4.00, time:-13.00 on predicate exec:(true)
 Inlined into get_a/1 which now has 6 size
...
Updating SSA:
Registering new PHI nodes in block #0
Registering new PHI nodes in block #2
Registering new PHI nodes in block #4
Updating SSA information for statement <retval> = *ptr_2(D);
Registering new PHI nodes in block #3
Updating SSA information for statement _4 = D.88.a;
Updating SSA information for statement return _4;
...

long int get_a (struct my_struct * ptr)
{
  struct my_struct D.88;
  long int _4;

  <bb 2> [local count: 1073741824]:
<L0>:
  <retval> = *ptr_2(D);
  _4 = D.88.a;
  return _4;
}

So the inliner is for some reason responsible for that.

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
Honza's investigation on IRC:

<honza_home> Looks like inlining did not understand the return slot
optimization, so it simply stores the value to <retslot> that is presumably now
a normal variable.
<honza_home> so I guess missing DECL_BY_REFERENCE flag on the return value
decl?

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this bug, and for the investigation.

After a couple of hours debugging, I think I've found the discrepancy between
cc1 and libgccjit, and have a candidate fix.

When tree-inline.c:remap_gimple_stmt is called to remap the stmt:
   <retval> = *ptr_2(D);
cc1 returns:
   D.1989 = *ptr_2(D);
whereas libgccjit returns this (copy):
   <retval> = *ptr_2(D);

Putting a breakpoint on remap_gimple_op_r and singlestepping (in both cc1 and
libgccjit, side by side) shows the discrepancy happens in
tree-inline.c:remap_gimple_op_r here:

   1036   else if (auto_var_in_fn_p (*tp, fn))

For cc1:

   (gdb) p auto_var_in_fn_p (*tp, fn)
   $28 = true

whereas for libgccjit:

   (gdb) p auto_var_in_fn_p (*tp, fn)
   $19 = false

and so libgccjit erroneously skips the remapping of locals vars and labels.

This turns out to be because the RESULT_DECL has a non-NULL DECL_CONTEXT in
cc1, set here by the C frontend:

(gdb) bt
#0  finish_function (end_loc=248384) at ../../src/gcc/c/c-decl.c:10271
#1  0x00000000009db5f7 in c_parser_declaration_or_fndef (parser=0x7ffff7ffbbd0,
fndef_ok=true, static_assert_ok=true, empty_ok=true, nested=false, 
    start_attr_ok=true, objc_foreach_object_declaration=0x0,
omp_declare_simd_clauses=0x0, have_attrs=false, attrs=<tree 0x0>, 
    oacc_routine_data=0x0, fallthru_attr_p=0x0) at
../../src/gcc/c/c-parser.c:2563
#2  0x00000000009d9925 in c_parser_external_declaration (parser=0x7ffff7ffbbd0)
at ../../src/gcc/c/c-parser.c:1779
#3  0x00000000009d941d in c_parser_translation_unit (parser=0x7ffff7ffbbd0) at
../../src/gcc/c/c-parser.c:1652
#4  0x0000000000a21267 in c_parse_file () at ../../src/gcc/c/c-parser.c:23280
#5  0x0000000000abd0ab in c_common_parse_file () at
../../src/gcc/c-family/c-opts.c:1238
#6  0x00000000010cab1a in compile_file () at ../../src/gcc/toplev.c:452
#7  0x000000000093eb7a in do_compile (no_backend=false) at
../../src/gcc/toplev.c:2156
#8  toplev::main (this=0x7fffffffde1a, argc=<optimized out>, argv=<optimized
out>) at ../../src/gcc/toplev.c:2308
#9  0x000000000270339d in main (argc=24, argv=0x7fffffffdf28) at
../../src/gcc/main.c:39
(gdb) list
10266     /* Must mark the RESULT_DECL as being in this function.  */
10267   
10268     if (DECL_RESULT (fndecl) && DECL_RESULT (fndecl) != error_mark_node)
10269       DECL_CONTEXT (DECL_RESULT (fndecl)) = fndecl;

whereas the RESULT_DECL has NULL for its DECL_CONTEXT.


On trying this patch:

--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -473,6 +473,7 @@ new_function (location *loc,
   DECL_ARTIFICIAL (resdecl) = 1;
   DECL_IGNORED_P (resdecl) = 1;
   DECL_RESULT (fndecl) = resdecl;
+  DECL_CONTEXT (resdecl) = fndecl;

   if (builtin_id)
     {

then remap_gimple_stmt in libgccjit correctly remaps the stmt to:

(gdb) call debug ($21)
D.88 = *ptr_2(D);

and I correctly get this output from the libgccjit testcase:

get_a(&s) is 1

i.e. the above patch seems to fix it.

I'm running some more thorough tests.

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:a2f4b4b76cdd0a4150e82e69fae4a70c54b523d2

commit r12-5903-ga2f4b4b76cdd0a4150e82e69fae4a70c54b523d2
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Dec 10 09:28:48 2021 -0500

    jit: set DECL_CONTEXT of RESULT_DECL [PR103562]

    libgccjit was failing to set the DECL_CONTEXT of function RESULT_DECLs,
    leading to them failing to be properly handled by the inlining machinery.
    Fixed thusly.

    gcc/jit/ChangeLog:
            PR jit/103562
            * jit-playback.c (gcc::jit::playback::context::new_function): Set
            DECL_CONTEXT of the result_decl.

    gcc/testsuite/ChangeLog:
            PR jit/103562
            * jit.dg/all-non-failing-tests.h: Add comment about...
            * jit.dg/test-pr103562.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

--- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed by the above commit on trunk for gcc 12.

Probably should backport this; keeping this open until that's done.

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by David Malcolm
<dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:1250dcee74985eaeacae3b85e5ec78937225d322

commit r11-9375-g1250dcee74985eaeacae3b85e5ec78937225d322
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Dec 10 09:28:48 2021 -0500

    jit: set DECL_CONTEXT of RESULT_DECL [PR103562]

    libgccjit was failing to set the DECL_CONTEXT of function RESULT_DECLs,
    leading to them failing to be properly handled by the inlining machinery.
    Fixed thusly.

    gcc/jit/ChangeLog:
            PR jit/103562
            * jit-playback.c (gcc::jit::playback::context::new_function): Set
            DECL_CONTEXT of the result_decl.

    gcc/testsuite/ChangeLog:
            PR jit/103562
            * jit.dg/all-non-failing-tests.h: Add comment about...
            * jit.dg/test-pr103562.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug jit/103562] Jitted code produces incorrect result when returning 3-member struct from internal function

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103562

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|---                         |11.3
         Resolution|---                         |FIXED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed for GCC 11.3.0.