Re: [PATCH] libgccjit: Handle truncation and extension for casts [PR 95498]

[Antoni Boucher <bouanto@zoho.com>]

[-- Attachment #1: Type: text/plain, Size: 3119 bytes --]

Hi.
Sorry for the long delay.

I attached the updated patch.

For your question, the current code already works with boolean and reals 
and casts between integers and pointers is currently not supported.

The tests now pass: I'm assuming I had to clean the build folder or 
something to make that work.

David, any more insight about whether this is the good solution for the 
problem?

Thanks.

On Tue, Jul 21, 2020 at 11:29:57PM +0200, Andrea Corallo wrote:
>Hi Antoni,
>
>a couple of nits and some thoughts.
>
>Antoni Boucher via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>
>> 2020-07-12  Antoni Boucher  <bouanto@zoho.com>
>>
>> gcc/jit/
>> 	PR target/95498
>> 	* jit-playback.c: Add support to handle truncation and extension
>                       ^^^
>                       here we usually add the function that gets
>modified, you can look at other changelog entries as example.
>
>> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
>> index 0fddf04da87..4f4a1080c36 100644
>> --- a/gcc/jit/jit-playback.c
>> +++ b/gcc/jit/jit-playback.c
>> @@ -61,22 +61,39 @@ along with GCC; see the file COPYING3.  If not see
>>
>>  /* gcc::jit::playback::context::build_cast uses the convert.h API,
>>     which in turn requires the frontend to provide a "convert"
>> -   function, apparently as a fallback.
>> -
>> -   Hence we provide this dummy one, with the requirement that any casts
>> -   are handled before reaching this.  */
>> +   function, apparently as a fallback for casts that can be simplified
>> +   (truncation, extension). */
>>  extern tree convert (tree type, tree expr);
>>
>>  tree
>>  convert (tree dst_type, tree expr)
>>  {
>> -  gcc_assert (gcc::jit::active_playback_ctxt);
>> -  gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
>> -  fprintf (stderr, "input expression:\n");
>> -  debug_tree (expr);
>> -  fprintf (stderr, "requested type:\n");
>> -  debug_tree (dst_type);
>> -  return error_mark_node;
>> +  tree t_ret = NULL;
>> +  t_ret = targetm.convert_to_type (dst_type, expr);
>> +  if (t_ret)
>> +      return t_ret;
>        ^^^
>        indent nit
>> +  enum tree_code dst_code = TREE_CODE (dst_type);
>> +  switch (dst_code)
>> +    {
>> +    case INTEGER_TYPE:
>> +    case ENUMERAL_TYPE:
>> +      t_ret = convert_to_integer (dst_type, expr);
>> +      goto maybe_fold;
>> +
>> +    default:
>> +      gcc_assert (gcc::jit::active_playback_ctxt);
>> +      gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
>> +      fprintf (stderr, "input expression:\n");
>> +      debug_tree (expr);
>> +      fprintf (stderr, "requested type:\n");
>> +      debug_tree (dst_type);
>> +      return error_mark_node;
>> +
>> +    maybe_fold:
>> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
>> +	t_ret = fold (t_ret);
>> +      return t_ret;
>> +    }
>>  }
>
>Looking at 'convert' at c-convert.c:66 the INTEGER_TYPE case here looks
>good, but given the set of casts we accept as input I guess we should
>handle also POINTER_TYPE, BOOLEAN_TYPE and REAL_TYPE.  What do you think
>about?
>
>Hope it helps
>
>Bests
>
>  Andrea

[-- Attachment #2: 0001-This-patch-handles-truncation-and-extension-for-cast.patch --]
[-- Type: text/plain, Size: 5411 bytes --]

From 3f42dc30d9276d7b830103479a3b03f5fc9027ec Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Sun, 5 Jul 2020 19:07:30 -0400
Subject: [PATCH] This patch handles truncation and extension for casts in jit.

2020-07-12  Antoni Boucher  <bouanto@zoho.com>

gcc/jit/
	PR target/95498
        * jit-playback.c (convert): Add support to handle truncation and
        extension in the convert function.

gcc/testsuite/
	PR target/95498
	* jit.dg/all-non-failing-tests.h: New test.
	* jit.dg/test-cast.c: New test.

Signed-off-by: Antoni Boucher <bouanto@zoho.com>
---
 gcc/jit/jit-playback.c                       | 39 ++++++++----
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
 gcc/testsuite/jit.dg/test-cast.c             | 66 ++++++++++++++++++++
 3 files changed, 104 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-cast.c

diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 4fac64dcab7..6cfe0371ec3 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -61,22 +61,39 @@ along with GCC; see the file COPYING3.  If not see
 
 /* gcc::jit::playback::context::build_cast uses the convert.h API,
    which in turn requires the frontend to provide a "convert"
-   function, apparently as a fallback.
-
-   Hence we provide this dummy one, with the requirement that any casts
-   are handled before reaching this.  */
+   function, apparently as a fallback for casts that can be simplified
+   (truncation, extension). */
 extern tree convert (tree type, tree expr);
 
 tree
 convert (tree dst_type, tree expr)
 {
-  gcc_assert (gcc::jit::active_playback_ctxt);
-  gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
-  fprintf (stderr, "input expression:\n");
-  debug_tree (expr);
-  fprintf (stderr, "requested type:\n");
-  debug_tree (dst_type);
-  return error_mark_node;
+  tree t_ret = NULL;
+  t_ret = targetm.convert_to_type (dst_type, expr);
+  if (t_ret)
+      return t_ret;
+  enum tree_code dst_code = TREE_CODE (dst_type);
+  switch (dst_code)
+    {
+    case INTEGER_TYPE:
+    case ENUMERAL_TYPE:
+      t_ret = convert_to_integer (dst_type, expr);
+      goto maybe_fold;
+
+    default:
+      gcc_assert (gcc::jit::active_playback_ctxt);
+      gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
+      fprintf (stderr, "input expression:\n");
+      debug_tree (expr);
+      fprintf (stderr, "requested type:\n");
+      debug_tree (dst_type);
+      return error_mark_node;
+
+    maybe_fold:
+      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
+	t_ret = fold (t_ret);
+      return t_ret;
+    }
 }
 
 namespace gcc {
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 4202eb7798b..84ef54a0386 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -98,6 +98,13 @@
 #undef create_code
 #undef verify_code
 
+/* test-cast.c */
+#define create_code create_code_cast
+#define verify_code verify_code_cast
+#include "test-cast.c"
+#undef create_code
+#undef verify_code
+
 /* test-compound-assignment.c */
 #define create_code create_code_compound_assignment
 #define verify_code verify_code_compound_assignment
@@ -361,6 +368,9 @@ const struct testcase testcases[] = {
   {"calling_internal_function",
    create_code_calling_internal_function,
    verify_code_calling_internal_function},
+  {"cast",
+   create_code_cast,
+   verify_code_cast},
   {"compound_assignment",
    create_code_compound_assignment,
    verify_code_compound_assignment},
diff --git a/gcc/testsuite/jit.dg/test-cast.c b/gcc/testsuite/jit.dg/test-cast.c
new file mode 100644
index 00000000000..2b1e385ae40
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-cast.c
@@ -0,0 +1,66 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Let's try to inject the equivalent of:
+char
+my_casts (int x)
+{
+   return (char)(long) x;
+}
+   */
+  gcc_jit_type *int_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+  gcc_jit_type *long_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_LONG);
+  gcc_jit_type *return_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_CHAR);
+
+  gcc_jit_param *x =
+    gcc_jit_context_new_param (
+      ctxt,
+      NULL,
+      int_type, "x");
+  gcc_jit_param *params[1] = {x};
+  gcc_jit_function *func =
+    gcc_jit_context_new_function (ctxt,
+				  NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  return_type,
+				  "my_casts",
+				  1, params, 0);
+
+  gcc_jit_block *initial =
+    gcc_jit_function_new_block (func, "initial");
+
+  gcc_jit_block_end_with_return(initial, NULL,
+    gcc_jit_context_new_cast(ctxt,
+        NULL,
+        gcc_jit_context_new_cast(ctxt,
+            NULL,
+            gcc_jit_param_as_rvalue(x),
+            long_type
+        ),
+        return_type
+    ));
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  typedef int (*my_casts_fn_type) (int);
+  CHECK_NON_NULL (result);
+  my_casts_fn_type my_casts =
+    (my_casts_fn_type)gcc_jit_result_get_code (result, "my_casts");
+  CHECK_NON_NULL (my_casts);
+  char val = my_casts (10);
+  note ("my_casts returned: %d", val);
+  CHECK_VALUE (val, 10);
+}
-- 
2.28.0