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

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

[Antoni Boucher]

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

Hello.

As mentioned in bug 95498, some conversions do not work. After 
investigation, it turns out that it's caused by multiple casts on an 
expression where it should do a truncation/extension.

I added a testcase, but for some reasons, the tests only pass when ran 
via `./testsuite/jit/test-cast.c.exe`, not when ran via `make check-jit 
RUNTESTFLAGS="-v -v -v  jit.exp=test-cast.c"`.
Furthermore, some other tests failed, but they also fail on master.

Also, I was under the impression that adding `STRIP_TYPE_NOPS (t_expr);` 
in `playback::context::build_cast` would be a better fix for this, but 
that doesn't fix the issue.
Am I missing something?

It's my first contribution to gcc, so I'd need help for fixing the tests 
and also a confirmation that this is the best way to fix this issue.

Thanks.

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

From 140333077a4c5f8ce80f7e5716eecc90781594fa 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: 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.
---
 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 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;
+  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 632ab8cfb2e..a1481699b16 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
@@ -354,6 +361,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.27.0

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

[David Malcolm]

On Sun, 2020-07-12 at 20:30 -0400, Antoni Boucher via Jit wrote:
> Hello.
> 
> As mentioned in bug 95498, some conversions do not work. After 
> investigation, it turns out that it's caused by multiple casts on an 
> expression where it should do a truncation/extension.

Thanks for investigating this.

> I added a testcase, but for some reasons, the tests only pass when
> ran 
> via `./testsuite/jit/test-cast.c.exe`, not when ran via `make check-
> jit 
> RUNTESTFLAGS="-v -v -v  jit.exp=test-cast.c"`.

That's weird.

It sounds like you're following the instructions on 
https://gcc.gnu.org/onlinedocs/jit/internals/index.html#running-the-test-suite

If you're not specifying LD_LIBRARY_PATH, it might be that when you
run 
  ./testsuite/jit/test-cast.c.exe
directly that the version of libgccjit.so that the exe gets dynamically
linked against might not be the one you expect (e.g. a distribution-
provided libgccjit.so, rather than the one in your build
directory).  Maybe that explains the differences in behavior?  (or
maybe somehow "make check-jit" is using the wrong DSO???).  You could
try exporting LD_DEBUG=files (or somesuch) to get info on which
libgccjit.so is being used in each manner of invoking the test.

There are some notes at the URL above on how to run a test under gdb.

If all else fails, add some printfs and look at the output from "make
check-jit", and look at jit.sum and jit.log (though you don't specify
how the tests fail to pass when run that way)

> Furthermore, some other tests failed, but they also fail on master.

Which tests are failing for you?  (and for what configuration triplet?)

> Also, I was under the impression that adding `STRIP_TYPE_NOPS
> (t_expr);` 
> in `playback::context::build_cast` would be a better fix for this,
> but 
> that doesn't fix the issue.
> Am I missing something?

I'm not sure yet what the best fix would be.  In general I try to mimic
C behavior as much as is reasonable, but off the top of my head I'm not
sure what the C frontend does here.

> It's my first contribution to gcc, so I'd need help for fixing the
> tests 
> and also a confirmation that this is the best way to fix this issue.

Thanks for posting the patch; I hope the above is helpful
Dave

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

[Andrea Corallo]

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

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

[Antoni Boucher]

[-- 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

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

[Antoni Boucher]

Hi.
I'd like to know what's the status of the review for this patch.
(Same for my other patch 96889: add some reflection functions in the jit 
C api)
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

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

[Tom Tromey]

>>>>> "Antoni" == Antoni Boucher via Gcc-patches <gcc-patches@gcc.gnu.org> writes:

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

Antoni> +  switch (dst_code)
Antoni> +    {
Antoni> +    case INTEGER_TYPE:
Antoni> +    case ENUMERAL_TYPE:
Antoni> +      t_ret = convert_to_integer (dst_type, expr);
Antoni> +      goto maybe_fold;
Antoni> +
Antoni> +    default:
Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
Antoni> +      fprintf (stderr, "input expression:\n");
Antoni> +      debug_tree (expr);
Antoni> +      fprintf (stderr, "requested type:\n");
Antoni> +      debug_tree (dst_type);
Antoni> +      return error_mark_node;
Antoni> +
Antoni> +    maybe_fold:
Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
Antoni> +	t_ret = fold (t_ret);
Antoni> +      return t_ret;

It seems weird to have a single 'goto' to maybe_fold, especially inside
a switch like this.

If you think the maybe_fold code won't be reused, then it should just be
hoisted up and the 'goto' removed.

On the other hand, if the maybe_fold code might be reused for some other
case, then I suppose I would have the case end with 'break' and then
have this code outside the switch.


In another message, you wrote:

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

I am curious why this wasn't supported.  It seems like something that
one might want to do.

thanks,
Tom

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

[Antoni Boucher]

Hi.
Thanks for your feedback!

See answers below:

On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
>>>>>> "Antoni" == Antoni Boucher via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>
>Antoni> gcc/jit/
>Antoni> 	PR target/95498
>Antoni> 	* jit-playback.c: Add support to handle truncation and extension
>Antoni> 	in the convert function.
>
>Antoni> +  switch (dst_code)
>Antoni> +    {
>Antoni> +    case INTEGER_TYPE:
>Antoni> +    case ENUMERAL_TYPE:
>Antoni> +      t_ret = convert_to_integer (dst_type, expr);
>Antoni> +      goto maybe_fold;
>Antoni> +
>Antoni> +    default:
>Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
>Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
>Antoni> +      fprintf (stderr, "input expression:\n");
>Antoni> +      debug_tree (expr);
>Antoni> +      fprintf (stderr, "requested type:\n");
>Antoni> +      debug_tree (dst_type);
>Antoni> +      return error_mark_node;
>Antoni> +
>Antoni> +    maybe_fold:
>Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
>Antoni> +	t_ret = fold (t_ret);
>Antoni> +      return t_ret;
>
>It seems weird to have a single 'goto' to maybe_fold, especially inside
>a switch like this.
>
>If you think the maybe_fold code won't be reused, then it should just be
>hoisted up and the 'goto' removed.

This actually depends on how the support for cast between integers and 
pointers will be implemented (see below).
If we will support truncating pointers (does that even make sense? and I 
guess we cannot extend a pointer unless we add the support for 
uint128_t), that label will be reused for that case.
Otherwise, it might not be reused.

So, please tell me which option to choose and I'll update my patch.

>On the other hand, if the maybe_fold code might be reused for some other
>case, then I suppose I would have the case end with 'break' and then
>have this code outside the switch.
>
>
>In another message, you wrote:
>
>Antoni> For your question, the current code already works with boolean and
>Antoni> reals and casts between integers and pointers is currently not
>Antoni> supported.
>
>I am curious why this wasn't supported.  It seems like something that
>one might want to do.

I have no idea as this is my first contribution to gcc.
But this would be indeed very useful and I opened an issue about this: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438

>thanks,
>Tom

Thanks!

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

[Martin Liška]

@David: PING

On 2/20/21 11:17 PM, Antoni Boucher via Gcc-patches wrote:
> Hi.
> Thanks for your feedback!
> 
> See answers below:
> 
> On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
>>>>>>> "Antoni" == Antoni Boucher via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>>
>> Antoni> gcc/jit/
>> Antoni>     PR target/95498
>> Antoni>     * jit-playback.c: Add support to handle truncation and extension
>> Antoni>     in the convert function.
>>
>> Antoni> +  switch (dst_code)
>> Antoni> +    {
>> Antoni> +    case INTEGER_TYPE:
>> Antoni> +    case ENUMERAL_TYPE:
>> Antoni> +      t_ret = convert_to_integer (dst_type, expr);
>> Antoni> +      goto maybe_fold;
>> Antoni> +
>> Antoni> +    default:
>> Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
>> Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
>> Antoni> +      fprintf (stderr, "input expression:\n");
>> Antoni> +      debug_tree (expr);
>> Antoni> +      fprintf (stderr, "requested type:\n");
>> Antoni> +      debug_tree (dst_type);
>> Antoni> +      return error_mark_node;
>> Antoni> +
>> Antoni> +    maybe_fold:
>> Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
>> Antoni> +    t_ret = fold (t_ret);
>> Antoni> +      return t_ret;
>>
>> It seems weird to have a single 'goto' to maybe_fold, especially inside
>> a switch like this.
>>
>> If you think the maybe_fold code won't be reused, then it should just be
>> hoisted up and the 'goto' removed.
> 
> This actually depends on how the support for cast between integers and pointers will be implemented (see below).
> If we will support truncating pointers (does that even make sense? and I guess we cannot extend a pointer unless we add the support for uint128_t), that label will be reused for that case.
> Otherwise, it might not be reused.
> 
> So, please tell me which option to choose and I'll update my patch.
> 
>> On the other hand, if the maybe_fold code might be reused for some other
>> case, then I suppose I would have the case end with 'break' and then
>> have this code outside the switch.
>>
>>
>> In another message, you wrote:
>>
>> Antoni> For your question, the current code already works with boolean and
>> Antoni> reals and casts between integers and pointers is currently not
>> Antoni> supported.
>>
>> I am curious why this wasn't supported.  It seems like something that
>> one might want to do.
> 
> I have no idea as this is my first contribution to gcc.
> But this would be indeed very useful and I opened an issue about this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> 
>> thanks,
>> Tom
> 
> Thanks!

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

[David Malcolm]

On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-patches
wrote:
> Hi.
> Thanks for your feedback!
> 

Sorry about the delay in responding.

In the past I was hesitant about adding more cast support to libgccjit
since I felt that the user could always just create a union to do the
cast.  Then I tried actually using the libgccjit API to do this, and
realized how much work it adds, so I now think we do want to support
casting more types.


> See answers below:
> 
> On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > gcc-patches@gcc.gnu.org> writes:
> > 
> > Antoni> gcc/jit/
> > Antoni>         PR target/95498
> > Antoni>         * jit-playback.c: Add support to handle truncation
> > and extension
> > Antoni>         in the convert function.
> > 
> > Antoni> +  switch (dst_code)
> > Antoni> +    {
> > Antoni> +    case INTEGER_TYPE:
> > Antoni> +    case ENUMERAL_TYPE:
> > Antoni> +      t_ret = convert_to_integer (dst_type, expr);
> > Antoni> +      goto maybe_fold;
> > Antoni> +
> > Antoni> +    default:
> > Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
> > Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL,
> > "unhandled conversion");
> > Antoni> +      fprintf (stderr, "input expression:\n");
> > Antoni> +      debug_tree (expr);
> > Antoni> +      fprintf (stderr, "requested type:\n");
> > Antoni> +      debug_tree (dst_type);
> > Antoni> +      return error_mark_node;
> > Antoni> +
> > Antoni> +    maybe_fold:
> > Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)

Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree code is
defined in c-family/c-common.def; how can nodes of that kind be created
outside of the c-family?

> > Antoni> +       t_ret = fold (t_ret);
> > Antoni> +      return t_ret;
> > 
> > It seems weird to have a single 'goto' to maybe_fold, especially
> > inside
> > a switch like this.
> > 
> > If you think the maybe_fold code won't be reused, then it should just
> > be
> > hoisted up and the 'goto' removed.
> 
> This actually depends on how the support for cast between integers and 
> pointers will be implemented (see below).
> If we will support truncating pointers (does that even make sense? and
> I 
> guess we cannot extend a pointer unless we add the support for 
> uint128_t), that label will be reused for that case.
> Otherwise, it might not be reused.
> 
> So, please tell me which option to choose and I'll update my patch.

FWIW I don't think we'll want to support truncating or extending
pointers.

> 
> > On the other hand, if the maybe_fold code might be reused for some
> > other
> > case, then I suppose I would have the case end with 'break' and then
> > have this code outside the switch.
> > 
> > 
> > In another message, you wrote:
> > 
> > Antoni> For your question, the current code already works with
> > boolean and
> > Antoni> reals and casts between integers and pointers is currently
> > not
> > Antoni> supported.
> > 
> > I am curious why this wasn't supported.  It seems like something that
> > one might want to do.
> 
> I have no idea as this is my first contribution to gcc.
> But this would be indeed very useful and I opened an issue about this: 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> 
> > thanks,
> > Tom
> 
> Thanks!
> 

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

[Antoni Boucher]

Thanks for your answer.

See my answers below:

Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-patches
> wrote:
> > Hi.
> > Thanks for your feedback!
> > 
> 
> Sorry about the delay in responding.
> 
> In the past I was hesitant about adding more cast support to libgccjit
> since I felt that the user could always just create a union to do the
> cast.  Then I tried actually using the libgccjit API to do this, and
> realized how much work it adds, so I now think we do want to support
> casting more types.
> 
> 
> > See answers below:
> > 
> > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > 
> > > Antoni> gcc/jit/
> > > Antoni>         PR target/95498
> > > Antoni>         * jit-playback.c: Add support to handle truncation
> > > and extension
> > > Antoni>         in the convert function.
> > > 
> > > Antoni> +  switch (dst_code)
> > > Antoni> +    {
> > > Antoni> +    case INTEGER_TYPE:
> > > Antoni> +    case ENUMERAL_TYPE:
> > > Antoni> +      t_ret = convert_to_integer (dst_type, expr);
> > > Antoni> +      goto maybe_fold;
> > > Antoni> +
> > > Antoni> +    default:
> > > Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
> > > Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL,
> > > "unhandled conversion");
> > > Antoni> +      fprintf (stderr, "input expression:\n");
> > > Antoni> +      debug_tree (expr);
> > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > Antoni> +      debug_tree (dst_type);
> > > Antoni> +      return error_mark_node;
> > > Antoni> +
> > > Antoni> +    maybe_fold:
> > > Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
> 
> Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree code is
> defined in c-family/c-common.def; how can nodes of that kind be created
> outside of the c-family?

I am not sure, but that seems like it's only created in c-family
indeed.
However, we do use it in libgccjit here:

https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180

> 
> > > Antoni> +       t_ret = fold (t_ret);
> > > Antoni> +      return t_ret;
> > > 
> > > It seems weird to have a single 'goto' to maybe_fold, especially
> > > inside
> > > a switch like this.
> > > 
> > > If you think the maybe_fold code won't be reused, then it should
> > > just
> > > be
> > > hoisted up and the 'goto' removed.
> > 
> > This actually depends on how the support for cast between integers
> > and 
> > pointers will be implemented (see below).
> > If we will support truncating pointers (does that even make sense?
> > and
> > I 
> > guess we cannot extend a pointer unless we add the support for 
> > uint128_t), that label will be reused for that case.
> > Otherwise, it might not be reused.
> > 
> > So, please tell me which option to choose and I'll update my patch.
> 
> FWIW I don't think we'll want to support truncating or extending
> pointers.

Ok, but do you think we'll want to support casts between integers and
pointers?
I opened an issue about this
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and would be
willing to do a patch for it eventually.

> > 
> > > On the other hand, if the maybe_fold code might be reused for some
> > > other
> > > case, then I suppose I would have the case end with 'break' and
> > > then
> > > have this code outside the switch.
> > > 
> > > 
> > > In another message, you wrote:
> > > 
> > > Antoni> For your question, the current code already works with
> > > boolean and
> > > Antoni> reals and casts between integers and pointers is currently
> > > not
> > > Antoni> supported.
> > > 
> > > I am curious why this wasn't supported.  It seems like something
> > > that
> > > one might want to do.
> > 
> > I have no idea as this is my first contribution to gcc.
> > But this would be indeed very useful and I opened an issue about
> > this: 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > 
> > > thanks,
> > > Tom
> > 
> > Thanks!
> > 
> 
> 

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

[David Malcolm]

On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> Thanks for your answer.
> 
> See my answers below:
> 
> Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-patches
> > wrote:
> > > Hi.
> > > Thanks for your feedback!
> > > 
> > 
> > Sorry about the delay in responding.
> > 
> > In the past I was hesitant about adding more cast support to
> > libgccjit
> > since I felt that the user could always just create a union to do
> > the
> > cast.  Then I tried actually using the libgccjit API to do this,
> > and
> > realized how much work it adds, so I now think we do want to
> > support
> > casting more types.
> > 
> > 
> > > See answers below:
> > > 
> > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > 
> > > > Antoni> gcc/jit/
> > > > Antoni>         PR target/95498
> > > > Antoni>         * jit-playback.c: Add support to handle
> > > > truncation
> > > > and extension
> > > > Antoni>         in the convert function.
> > > > 
> > > > Antoni> +  switch (dst_code)
> > > > Antoni> +    {
> > > > Antoni> +    case INTEGER_TYPE:
> > > > Antoni> +    case ENUMERAL_TYPE:
> > > > Antoni> +      t_ret = convert_to_integer (dst_type, expr);
> > > > Antoni> +      goto maybe_fold;
> > > > Antoni> +
> > > > Antoni> +    default:
> > > > Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
> > > > Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL,
> > > > "unhandled conversion");
> > > > Antoni> +      fprintf (stderr, "input expression:\n");
> > > > Antoni> +      debug_tree (expr);
> > > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > > Antoni> +      debug_tree (dst_type);
> > > > Antoni> +      return error_mark_node;
> > > > Antoni> +
> > > > Antoni> +    maybe_fold:
> > > > Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
> > 
> > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree code is
> > defined in c-family/c-common.def; how can nodes of that kind be
> > created
> > outside of the c-family?
> 
> I am not sure, but that seems like it's only created in c-family
> indeed.
> However, we do use it in libgccjit here:
> 
> https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> 
> > 
> > > > Antoni> +       t_ret = fold (t_ret);
> > > > Antoni> +      return t_ret;
> > > > 
> > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > especially
> > > > inside
> > > > a switch like this.
> > > > 
> > > > If you think the maybe_fold code won't be reused, then it
> > > > should
> > > > just
> > > > be
> > > > hoisted up and the 'goto' removed.
> > > 
> > > This actually depends on how the support for cast between
> > > integers
> > > and 
> > > pointers will be implemented (see below).
> > > If we will support truncating pointers (does that even make
> > > sense?
> > > and
> > > I 
> > > guess we cannot extend a pointer unless we add the support for 
> > > uint128_t), that label will be reused for that case.
> > > Otherwise, it might not be reused.
> > > 
> > > So, please tell me which option to choose and I'll update my
> > > patch.
> > 
> > FWIW I don't think we'll want to support truncating or extending
> > pointers.
> 
> Ok, but do you think we'll want to support casts between integers and
> pointers?

Yes, though we probably want to reject truncating a pointer into a
smaller integer type.

> I opened an issue about this
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and would be
> willing to do a patch for it eventually.
> 
> > > 
> > > > On the other hand, if the maybe_fold code might be reused for
> > > > some
> > > > other
> > > > case, then I suppose I would have the case end with 'break' and
> > > > then
> > > > have this code outside the switch.
> > > > 
> > > > 
> > > > In another message, you wrote:
> > > > 
> > > > Antoni> For your question, the current code already works with
> > > > boolean and
> > > > Antoni> reals and casts between integers and pointers is
> > > > currently
> > > > not
> > > > Antoni> supported.
> > > > 
> > > > I am curious why this wasn't supported.  It seems like
> > > > something
> > > > that
> > > > one might want to do.
> > > 
> > > I have no idea as this is my first contribution to gcc.
> > > But this would be indeed very useful and I opened an issue about
> > > this: 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > 
> > > > thanks,
> > > > Tom
> > > 
> > > Thanks!
> > > 
> > 
> > 
> 
> 

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

[Antoni Boucher]

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

I updated the patch according to the comments by Tom Tromey.

There's one question left about your question regarding
C_MAYBE_CONST_EXPR, David:

I am not sure if we can get a C_MAYBE_CONST_EXPR from libgccjit, and it
indeed seems like it's only created in c-family.
However, we do use it in libgccjit here:
https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180

I tried removing the condition `if (TREE_CODE (t_ret) !=
C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.

That code was copied from here:
https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
and might not be needed in libgccjit.

Should I just remove the condition, then?

Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > Thanks for your answer.
> > 
> > See my answers below:
> > 
> > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-patches
> > > wrote:
> > > > Hi.
> > > > Thanks for your feedback!
> > > > 
> > > 
> > > Sorry about the delay in responding.
> > > 
> > > In the past I was hesitant about adding more cast support to
> > > libgccjit
> > > since I felt that the user could always just create a union to do
> > > the
> > > cast.  Then I tried actually using the libgccjit API to do this,
> > > and
> > > realized how much work it adds, so I now think we do want to
> > > support
> > > casting more types.
> > > 
> > > 
> > > > See answers below:
> > > > 
> > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > 
> > > > > Antoni> gcc/jit/
> > > > > Antoni>         PR target/95498
> > > > > Antoni>         * jit-playback.c: Add support to handle
> > > > > truncation
> > > > > and extension
> > > > > Antoni>         in the convert function.
> > > > > 
> > > > > Antoni> +  switch (dst_code)
> > > > > Antoni> +    {
> > > > > Antoni> +    case INTEGER_TYPE:
> > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > Antoni> +      t_ret = convert_to_integer (dst_type, expr);
> > > > > Antoni> +      goto maybe_fold;
> > > > > Antoni> +
> > > > > Antoni> +    default:
> > > > > Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
> > > > > Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL,
> > > > > "unhandled conversion");
> > > > > Antoni> +      fprintf (stderr, "input expression:\n");
> > > > > Antoni> +      debug_tree (expr);
> > > > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > > > Antoni> +      debug_tree (dst_type);
> > > > > Antoni> +      return error_mark_node;
> > > > > Antoni> +
> > > > > Antoni> +    maybe_fold:
> > > > > Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
> > > 
> > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree code is
> > > defined in c-family/c-common.def; how can nodes of that kind be
> > > created
> > > outside of the c-family?
> > 
> > I am not sure, but that seems like it's only created in c-family
> > indeed.
> > However, we do use it in libgccjit here:
> > 
> > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > 
> > > 
> > > > > Antoni> +       t_ret = fold (t_ret);
> > > > > Antoni> +      return t_ret;
> > > > > 
> > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > especially
> > > > > inside
> > > > > a switch like this.
> > > > > 
> > > > > If you think the maybe_fold code won't be reused, then it
> > > > > should
> > > > > just
> > > > > be
> > > > > hoisted up and the 'goto' removed.
> > > > 
> > > > This actually depends on how the support for cast between
> > > > integers
> > > > and 
> > > > pointers will be implemented (see below).
> > > > If we will support truncating pointers (does that even make
> > > > sense?
> > > > and
> > > > I 
> > > > guess we cannot extend a pointer unless we add the support for 
> > > > uint128_t), that label will be reused for that case.
> > > > Otherwise, it might not be reused.
> > > > 
> > > > So, please tell me which option to choose and I'll update my
> > > > patch.
> > > 
> > > FWIW I don't think we'll want to support truncating or extending
> > > pointers.
> > 
> > Ok, but do you think we'll want to support casts between integers and
> > pointers?
> 
> Yes, though we probably want to reject truncating a pointer into a
> smaller integer type.
> 
> > I opened an issue about this
> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and would be
> > willing to do a patch for it eventually.
> > 
> > > > 
> > > > > On the other hand, if the maybe_fold code might be reused for
> > > > > some
> > > > > other
> > > > > case, then I suppose I would have the case end with 'break' and
> > > > > then
> > > > > have this code outside the switch.
> > > > > 
> > > > > 
> > > > > In another message, you wrote:
> > > > > 
> > > > > Antoni> For your question, the current code already works with
> > > > > boolean and
> > > > > Antoni> reals and casts between integers and pointers is
> > > > > currently
> > > > > not
> > > > > Antoni> supported.
> > > > > 
> > > > > I am curious why this wasn't supported.  It seems like
> > > > > something
> > > > > that
> > > > > one might want to do.
> > > > 
> > > > I have no idea as this is my first contribution to gcc.
> > > > But this would be indeed very useful and I opened an issue about
> > > > this: 
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > > 
> > > > > thanks,
> > > > > Tom
> > > > 
> > > > Thanks!
> > > > 
> > > 
> > > 
> > 
> > 
> 
> 


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

From 964a972b25d7e3a6238046403bbd9cb336724980 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                       | 36 +++++++----
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
 gcc/testsuite/jit.dg/test-cast.c             | 66 ++++++++++++++++++++
 3 files changed, 101 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..ce990bd18c6 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -61,22 +61,36 @@ 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);
+      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
+	t_ret = fold (t_ret);
+      return t_ret;
+
+    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;
+    }
 }
 
 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.31.1

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

[David Malcolm]

On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> I updated the patch according to the comments by Tom Tromey.
> 
> There's one question left about your question regarding
> C_MAYBE_CONST_EXPR, David:
> 
> I am not sure if we can get a C_MAYBE_CONST_EXPR from libgccjit, and
> it
> indeed seems like it's only created in c-family.
> However, we do use it in libgccjit here:
> https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> 
> I tried removing the condition `if (TREE_CODE (t_ret) !=
> C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.
> 
> That code was copied from here:
> https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> and might not be needed in libgccjit.
> 
> Should I just remove the condition, then?

I think so.

Thanks
Dave

> 
> Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > Thanks for your answer.
> > > 
> > > See my answers below:
> > > 
> > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-
> > > > patches
> > > > wrote:
> > > > > Hi.
> > > > > Thanks for your feedback!
> > > > > 
> > > > 
> > > > Sorry about the delay in responding.
> > > > 
> > > > In the past I was hesitant about adding more cast support to
> > > > libgccjit
> > > > since I felt that the user could always just create a union to
> > > > do
> > > > the
> > > > cast.  Then I tried actually using the libgccjit API to do
> > > > this,
> > > > and
> > > > realized how much work it adds, so I now think we do want to
> > > > support
> > > > casting more types.
> > > > 
> > > > 
> > > > > See answers below:
> > > > > 
> > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > 
> > > > > > Antoni> gcc/jit/
> > > > > > Antoni>         PR target/95498
> > > > > > Antoni>         * jit-playback.c: Add support to handle
> > > > > > truncation
> > > > > > and extension
> > > > > > Antoni>         in the convert function.
> > > > > > 
> > > > > > Antoni> +  switch (dst_code)
> > > > > > Antoni> +    {
> > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > Antoni> +      t_ret = convert_to_integer (dst_type, expr);
> > > > > > Antoni> +      goto maybe_fold;
> > > > > > Antoni> +
> > > > > > Antoni> +    default:
> > > > > > Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
> > > > > > Antoni> +      gcc::jit::active_playback_ctxt->add_error
> > > > > > (NULL,
> > > > > > "unhandled conversion");
> > > > > > Antoni> +      fprintf (stderr, "input expression:\n");
> > > > > > Antoni> +      debug_tree (expr);
> > > > > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > > > > Antoni> +      debug_tree (dst_type);
> > > > > > Antoni> +      return error_mark_node;
> > > > > > Antoni> +
> > > > > > Antoni> +    maybe_fold:
> > > > > > Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
> > > > 
> > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree code
> > > > is
> > > > defined in c-family/c-common.def; how can nodes of that kind be
> > > > created
> > > > outside of the c-family?
> > > 
> > > I am not sure, but that seems like it's only created in c-family
> > > indeed.
> > > However, we do use it in libgccjit here:
> > > 
> > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > 
> > > > 
> > > > > > Antoni> +       t_ret = fold (t_ret);
> > > > > > Antoni> +      return t_ret;
> > > > > > 
> > > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > > especially
> > > > > > inside
> > > > > > a switch like this.
> > > > > > 
> > > > > > If you think the maybe_fold code won't be reused, then it
> > > > > > should
> > > > > > just
> > > > > > be
> > > > > > hoisted up and the 'goto' removed.
> > > > > 
> > > > > This actually depends on how the support for cast between
> > > > > integers
> > > > > and 
> > > > > pointers will be implemented (see below).
> > > > > If we will support truncating pointers (does that even make
> > > > > sense?
> > > > > and
> > > > > I 
> > > > > guess we cannot extend a pointer unless we add the support
> > > > > for 
> > > > > uint128_t), that label will be reused for that case.
> > > > > Otherwise, it might not be reused.
> > > > > 
> > > > > So, please tell me which option to choose and I'll update my
> > > > > patch.
> > > > 
> > > > FWIW I don't think we'll want to support truncating or
> > > > extending
> > > > pointers.
> > > 
> > > Ok, but do you think we'll want to support casts between integers
> > > and
> > > pointers?
> > 
> > Yes, though we probably want to reject truncating a pointer into a
> > smaller integer type.
> > 
> > > I opened an issue about this
> > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and would be
> > > willing to do a patch for it eventually.
> > > 
> > > > > 
> > > > > > On the other hand, if the maybe_fold code might be reused
> > > > > > for
> > > > > > some
> > > > > > other
> > > > > > case, then I suppose I would have the case end with 'break'
> > > > > > and
> > > > > > then
> > > > > > have this code outside the switch.
> > > > > > 
> > > > > > 
> > > > > > In another message, you wrote:
> > > > > > 
> > > > > > Antoni> For your question, the current code already works
> > > > > > with
> > > > > > boolean and
> > > > > > Antoni> reals and casts between integers and pointers is
> > > > > > currently
> > > > > > not
> > > > > > Antoni> supported.
> > > > > > 
> > > > > > I am curious why this wasn't supported.  It seems like
> > > > > > something
> > > > > > that
> > > > > > one might want to do.
> > > > > 
> > > > > I have no idea as this is my first contribution to gcc.
> > > > > But this would be indeed very useful and I opened an issue
> > > > > about
> > > > > this: 
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > > > 
> > > > > > thanks,
> > > > > > Tom
> > > > > 
> > > > > Thanks!
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> 

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

[Antoni Boucher]

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

Here's the patch with the condition removed.
I believe everything is now fixed.
Thanks!

Le jeudi 27 mai 2021 à 18:21 -0400, David Malcolm a écrit :
> On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> > I updated the patch according to the comments by Tom Tromey.
> > 
> > There's one question left about your question regarding
> > C_MAYBE_CONST_EXPR, David:
> > 
> > I am not sure if we can get a C_MAYBE_CONST_EXPR from libgccjit,
> > and
> > it
> > indeed seems like it's only created in c-family.
> > However, we do use it in libgccjit here:
> > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > 
> > I tried removing the condition `if (TREE_CODE (t_ret) !=
> > C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.
> > 
> > That code was copied from here:
> > https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> > and might not be needed in libgccjit.
> > 
> > Should I just remove the condition, then?
> 
> I think so.
> 
> Thanks
> Dave
> 
> > 
> > Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > > Thanks for your answer.
> > > > 
> > > > See my answers below:
> > > > 
> > > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-
> > > > > patches
> > > > > wrote:
> > > > > > Hi.
> > > > > > Thanks for your feedback!
> > > > > > 
> > > > > 
> > > > > Sorry about the delay in responding.
> > > > > 
> > > > > In the past I was hesitant about adding more cast support to
> > > > > libgccjit
> > > > > since I felt that the user could always just create a union
> > > > > to
> > > > > do
> > > > > the
> > > > > cast.  Then I tried actually using the libgccjit API to do
> > > > > this,
> > > > > and
> > > > > realized how much work it adds, so I now think we do want to
> > > > > support
> > > > > casting more types.
> > > > > 
> > > > > 
> > > > > > See answers below:
> > > > > > 
> > > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > > 
> > > > > > > Antoni> gcc/jit/
> > > > > > > Antoni>         PR target/95498
> > > > > > > Antoni>         * jit-playback.c: Add support to handle
> > > > > > > truncation
> > > > > > > and extension
> > > > > > > Antoni>         in the convert function.
> > > > > > > 
> > > > > > > Antoni> +  switch (dst_code)
> > > > > > > Antoni> +    {
> > > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > > Antoni> +      t_ret = convert_to_integer (dst_type,
> > > > > > > expr);
> > > > > > > Antoni> +      goto maybe_fold;
> > > > > > > Antoni> +
> > > > > > > Antoni> +    default:
> > > > > > > Antoni> +      gcc_assert
> > > > > > > (gcc::jit::active_playback_ctxt);
> > > > > > > Antoni> +      gcc::jit::active_playback_ctxt->add_error
> > > > > > > (NULL,
> > > > > > > "unhandled conversion");
> > > > > > > Antoni> +      fprintf (stderr, "input expression:\n");
> > > > > > > Antoni> +      debug_tree (expr);
> > > > > > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > > > > > Antoni> +      debug_tree (dst_type);
> > > > > > > Antoni> +      return error_mark_node;
> > > > > > > Antoni> +
> > > > > > > Antoni> +    maybe_fold:
> > > > > > > Antoni> +      if (TREE_CODE (t_ret) !=
> > > > > > > C_MAYBE_CONST_EXPR)
> > > > > 
> > > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree
> > > > > code
> > > > > is
> > > > > defined in c-family/c-common.def; how can nodes of that kind
> > > > > be
> > > > > created
> > > > > outside of the c-family?
> > > > 
> > > > I am not sure, but that seems like it's only created in c-
> > > > family
> > > > indeed.
> > > > However, we do use it in libgccjit here:
> > > > 
> > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > 
> > > > > 
> > > > > > > Antoni> +       t_ret = fold (t_ret);
> > > > > > > Antoni> +      return t_ret;
> > > > > > > 
> > > > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > > > especially
> > > > > > > inside
> > > > > > > a switch like this.
> > > > > > > 
> > > > > > > If you think the maybe_fold code won't be reused, then it
> > > > > > > should
> > > > > > > just
> > > > > > > be
> > > > > > > hoisted up and the 'goto' removed.
> > > > > > 
> > > > > > This actually depends on how the support for cast between
> > > > > > integers
> > > > > > and 
> > > > > > pointers will be implemented (see below).
> > > > > > If we will support truncating pointers (does that even make
> > > > > > sense?
> > > > > > and
> > > > > > I 
> > > > > > guess we cannot extend a pointer unless we add the support
> > > > > > for 
> > > > > > uint128_t), that label will be reused for that case.
> > > > > > Otherwise, it might not be reused.
> > > > > > 
> > > > > > So, please tell me which option to choose and I'll update
> > > > > > my
> > > > > > patch.
> > > > > 
> > > > > FWIW I don't think we'll want to support truncating or
> > > > > extending
> > > > > pointers.
> > > > 
> > > > Ok, but do you think we'll want to support casts between
> > > > integers
> > > > and
> > > > pointers?
> > > 
> > > Yes, though we probably want to reject truncating a pointer into
> > > a
> > > smaller integer type.
> > > 
> > > > I opened an issue about this
> > > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and would
> > > > be
> > > > willing to do a patch for it eventually.
> > > > 
> > > > > > 
> > > > > > > On the other hand, if the maybe_fold code might be reused
> > > > > > > for
> > > > > > > some
> > > > > > > other
> > > > > > > case, then I suppose I would have the case end with
> > > > > > > 'break'
> > > > > > > and
> > > > > > > then
> > > > > > > have this code outside the switch.
> > > > > > > 
> > > > > > > 
> > > > > > > In another message, you wrote:
> > > > > > > 
> > > > > > > Antoni> For your question, the current code already works
> > > > > > > with
> > > > > > > boolean and
> > > > > > > Antoni> reals and casts between integers and pointers is
> > > > > > > currently
> > > > > > > not
> > > > > > > Antoni> supported.
> > > > > > > 
> > > > > > > I am curious why this wasn't supported.  It seems like
> > > > > > > something
> > > > > > > that
> > > > > > > one might want to do.
> > > > > > 
> > > > > > I have no idea as this is my first contribution to gcc.
> > > > > > But this would be indeed very useful and I opened an issue
> > > > > > about
> > > > > > this: 
> > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > > > > 
> > > > > > > thanks,
> > > > > > > Tom
> > > > > > 
> > > > > > Thanks!
> > > > > > 
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> 
> 


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

From 18b9b05268c53368631937e9bdf3086aab8883dd 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                       | 33 ++++++----
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
 gcc/testsuite/jit.dg/test-cast.c             | 66 ++++++++++++++++++++
 3 files changed, 98 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 c6136301243..d26f08bfc1b 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -62,22 +62,33 @@ 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:
+      return fold (convert_to_integer (dst_type, expr));
+
+    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;
+    }
 }
 
 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.31.1

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

[David Malcolm]

On Thu, 2021-05-27 at 21:22 -0400, Antoni Boucher wrote:
> Here's the patch with the condition removed.
> I believe everything is now fixed.
> Thanks!

Thanks; this looks good to me.  Is this the latest version of the
patch; would you like me to apply it?

Dave

> 
> Le jeudi 27 mai 2021 à 18:21 -0400, David Malcolm a écrit :
> > On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> > > I updated the patch according to the comments by Tom Tromey.
> > > 
> > > There's one question left about your question regarding
> > > C_MAYBE_CONST_EXPR, David:
> > > 
> > > I am not sure if we can get a C_MAYBE_CONST_EXPR from libgccjit,
> > > and
> > > it
> > > indeed seems like it's only created in c-family.
> > > However, we do use it in libgccjit here:
> > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > 
> > > I tried removing the condition `if (TREE_CODE (t_ret) !=
> > > C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.
> > > 
> > > That code was copied from here:
> > > https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> > > and might not be needed in libgccjit.
> > > 
> > > Should I just remove the condition, then?
> > 
> > I think so.
> > 
> > Thanks
> > Dave
> > 
> > > 
> > > Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > > > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > > > Thanks for your answer.
> > > > > 
> > > > > See my answers below:
> > > > > 
> > > > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-
> > > > > > patches
> > > > > > wrote:
> > > > > > > Hi.
> > > > > > > Thanks for your feedback!
> > > > > > > 
> > > > > > 
> > > > > > Sorry about the delay in responding.
> > > > > > 
> > > > > > In the past I was hesitant about adding more cast support
> > > > > > to
> > > > > > libgccjit
> > > > > > since I felt that the user could always just create a union
> > > > > > to
> > > > > > do
> > > > > > the
> > > > > > cast.  Then I tried actually using the libgccjit API to do
> > > > > > this,
> > > > > > and
> > > > > > realized how much work it adds, so I now think we do want
> > > > > > to
> > > > > > support
> > > > > > casting more types.
> > > > > > 
> > > > > > 
> > > > > > > See answers below:
> > > > > > > 
> > > > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey
> > > > > > > wrote:
> > > > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches
> > > > > > > > > > > > > <   
> > > > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > > > 
> > > > > > > > Antoni> gcc/jit/
> > > > > > > > Antoni>         PR target/95498
> > > > > > > > Antoni>         * jit-playback.c: Add support to handle
> > > > > > > > truncation
> > > > > > > > and extension
> > > > > > > > Antoni>         in the convert function.
> > > > > > > > 
> > > > > > > > Antoni> +  switch (dst_code)
> > > > > > > > Antoni> +    {
> > > > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > > > Antoni> +      t_ret = convert_to_integer (dst_type,
> > > > > > > > expr);
> > > > > > > > Antoni> +      goto maybe_fold;
> > > > > > > > Antoni> +
> > > > > > > > Antoni> +    default:
> > > > > > > > Antoni> +      gcc_assert
> > > > > > > > (gcc::jit::active_playback_ctxt);
> > > > > > > > Antoni> +      gcc::jit::active_playback_ctxt-
> > > > > > > > >add_error
> > > > > > > > (NULL,
> > > > > > > > "unhandled conversion");
> > > > > > > > Antoni> +      fprintf (stderr, "input expression:\n");
> > > > > > > > Antoni> +      debug_tree (expr);
> > > > > > > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > > > > > > Antoni> +      debug_tree (dst_type);
> > > > > > > > Antoni> +      return error_mark_node;
> > > > > > > > Antoni> +
> > > > > > > > Antoni> +    maybe_fold:
> > > > > > > > Antoni> +      if (TREE_CODE (t_ret) !=
> > > > > > > > C_MAYBE_CONST_EXPR)
> > > > > > 
> > > > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree
> > > > > > code
> > > > > > is
> > > > > > defined in c-family/c-common.def; how can nodes of that
> > > > > > kind
> > > > > > be
> > > > > > created
> > > > > > outside of the c-family?
> > > > > 
> > > > > I am not sure, but that seems like it's only created in c-
> > > > > family
> > > > > indeed.
> > > > > However, we do use it in libgccjit here:
> > > > > 
> > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > > 
> > > > > > 
> > > > > > > > Antoni> +       t_ret = fold (t_ret);
> > > > > > > > Antoni> +      return t_ret;
> > > > > > > > 
> > > > > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > > > > especially
> > > > > > > > inside
> > > > > > > > a switch like this.
> > > > > > > > 
> > > > > > > > If you think the maybe_fold code won't be reused, then
> > > > > > > > it
> > > > > > > > should
> > > > > > > > just
> > > > > > > > be
> > > > > > > > hoisted up and the 'goto' removed.
> > > > > > > 
> > > > > > > This actually depends on how the support for cast between
> > > > > > > integers
> > > > > > > and 
> > > > > > > pointers will be implemented (see below).
> > > > > > > If we will support truncating pointers (does that even
> > > > > > > make
> > > > > > > sense?
> > > > > > > and
> > > > > > > I 
> > > > > > > guess we cannot extend a pointer unless we add the
> > > > > > > support
> > > > > > > for 
> > > > > > > uint128_t), that label will be reused for that case.
> > > > > > > Otherwise, it might not be reused.
> > > > > > > 
> > > > > > > So, please tell me which option to choose and I'll update
> > > > > > > my
> > > > > > > patch.
> > > > > > 
> > > > > > FWIW I don't think we'll want to support truncating or
> > > > > > extending
> > > > > > pointers.
> > > > > 
> > > > > Ok, but do you think we'll want to support casts between
> > > > > integers
> > > > > and
> > > > > pointers?
> > > > 
> > > > Yes, though we probably want to reject truncating a pointer
> > > > into
> > > > a
> > > > smaller integer type.
> > > > 
> > > > > I opened an issue about this
> > > > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and
> > > > > would
> > > > > be
> > > > > willing to do a patch for it eventually.
> > > > > 
> > > > > > > 
> > > > > > > > On the other hand, if the maybe_fold code might be
> > > > > > > > reused
> > > > > > > > for
> > > > > > > > some
> > > > > > > > other
> > > > > > > > case, then I suppose I would have the case end with
> > > > > > > > 'break'
> > > > > > > > and
> > > > > > > > then
> > > > > > > > have this code outside the switch.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > In another message, you wrote:
> > > > > > > > 
> > > > > > > > Antoni> For your question, the current code already
> > > > > > > > works
> > > > > > > > with
> > > > > > > > boolean and
> > > > > > > > Antoni> reals and casts between integers and pointers
> > > > > > > > is
> > > > > > > > currently
> > > > > > > > not
> > > > > > > > Antoni> supported.
> > > > > > > > 
> > > > > > > > I am curious why this wasn't supported.  It seems like
> > > > > > > > something
> > > > > > > > that
> > > > > > > > one might want to do.
> > > > > > > 
> > > > > > > I have no idea as this is my first contribution to gcc.
> > > > > > > But this would be indeed very useful and I opened an
> > > > > > > issue
> > > > > > > about
> > > > > > > this: 
> > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > > > > > 
> > > > > > > > thanks,
> > > > > > > > Tom
> > > > > > > 
> > > > > > > Thanks!
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > 
> > 
> 

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

[Antoni Boucher]

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

I'm sending the patch once again for review.

As it's the first time I'll land a patch, I'm not sure what needs to be
done when it's approved.
Do I just commit it to the master branch directly?

Thanks!

Le vendredi 11 juin 2021 à 13:49 -0400, David Malcolm a écrit :
> On Thu, 2021-05-27 at 21:22 -0400, Antoni Boucher wrote:
> > Here's the patch with the condition removed.
> > I believe everything is now fixed.
> > Thanks!
> 
> Thanks; this looks good to me.  Is this the latest version of the
> patch; would you like me to apply it?
> 
> Dave
> 
> > 
> > Le jeudi 27 mai 2021 à 18:21 -0400, David Malcolm a écrit :
> > > On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> > > > I updated the patch according to the comments by Tom Tromey.
> > > > 
> > > > There's one question left about your question regarding
> > > > C_MAYBE_CONST_EXPR, David:
> > > > 
> > > > I am not sure if we can get a C_MAYBE_CONST_EXPR from
> > > > libgccjit,
> > > > and
> > > > it
> > > > indeed seems like it's only created in c-family.
> > > > However, we do use it in libgccjit here:
> > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > 
> > > > I tried removing the condition `if (TREE_CODE (t_ret) !=
> > > > C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.
> > > > 
> > > > That code was copied from here:
> > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> > > > and might not be needed in libgccjit.
> > > > 
> > > > Should I just remove the condition, then?
> > > 
> > > I think so.
> > > 
> > > Thanks
> > > Dave
> > > 
> > > > 
> > > > Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > > > > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > > > > Thanks for your answer.
> > > > > > 
> > > > > > See my answers below:
> > > > > > 
> > > > > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > > > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via
> > > > > > > Gcc-
> > > > > > > patches
> > > > > > > wrote:
> > > > > > > > Hi.
> > > > > > > > Thanks for your feedback!
> > > > > > > > 
> > > > > > > 
> > > > > > > Sorry about the delay in responding.
> > > > > > > 
> > > > > > > In the past I was hesitant about adding more cast support
> > > > > > > to
> > > > > > > libgccjit
> > > > > > > since I felt that the user could always just create a
> > > > > > > union
> > > > > > > to
> > > > > > > do
> > > > > > > the
> > > > > > > cast.  Then I tried actually using the libgccjit API to
> > > > > > > do
> > > > > > > this,
> > > > > > > and
> > > > > > > realized how much work it adds, so I now think we do want
> > > > > > > to
> > > > > > > support
> > > > > > > casting more types.
> > > > > > > 
> > > > > > > 
> > > > > > > > See answers below:
> > > > > > > > 
> > > > > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey
> > > > > > > > wrote:
> > > > > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches
> > > > > > > > > > > > > > <   
> > > > > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > > > > 
> > > > > > > > > Antoni> gcc/jit/
> > > > > > > > > Antoni>         PR target/95498
> > > > > > > > > Antoni>         * jit-playback.c: Add support to
> > > > > > > > > handle
> > > > > > > > > truncation
> > > > > > > > > and extension
> > > > > > > > > Antoni>         in the convert function.
> > > > > > > > > 
> > > > > > > > > Antoni> +  switch (dst_code)
> > > > > > > > > Antoni> +    {
> > > > > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > > > > Antoni> +      t_ret = convert_to_integer (dst_type,
> > > > > > > > > expr);
> > > > > > > > > Antoni> +      goto maybe_fold;
> > > > > > > > > Antoni> +
> > > > > > > > > Antoni> +    default:
> > > > > > > > > Antoni> +      gcc_assert
> > > > > > > > > (gcc::jit::active_playback_ctxt);
> > > > > > > > > Antoni> +      gcc::jit::active_playback_ctxt-
> > > > > > > > > > add_error
> > > > > > > > > (NULL,
> > > > > > > > > "unhandled conversion");
> > > > > > > > > Antoni> +      fprintf (stderr, "input
> > > > > > > > > expression:\n");
> > > > > > > > > Antoni> +      debug_tree (expr);
> > > > > > > > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > > > > > > > Antoni> +      debug_tree (dst_type);
> > > > > > > > > Antoni> +      return error_mark_node;
> > > > > > > > > Antoni> +
> > > > > > > > > Antoni> +    maybe_fold:
> > > > > > > > > Antoni> +      if (TREE_CODE (t_ret) !=
> > > > > > > > > C_MAYBE_CONST_EXPR)
> > > > > > > 
> > > > > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That
> > > > > > > tree
> > > > > > > code
> > > > > > > is
> > > > > > > defined in c-family/c-common.def; how can nodes of that
> > > > > > > kind
> > > > > > > be
> > > > > > > created
> > > > > > > outside of the c-family?
> > > > > > 
> > > > > > I am not sure, but that seems like it's only created in c-
> > > > > > family
> > > > > > indeed.
> > > > > > However, we do use it in libgccjit here:
> > > > > > 
> > > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > > > 
> > > > > > > 
> > > > > > > > > Antoni> +       t_ret = fold (t_ret);
> > > > > > > > > Antoni> +      return t_ret;
> > > > > > > > > 
> > > > > > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > > > > > especially
> > > > > > > > > inside
> > > > > > > > > a switch like this.
> > > > > > > > > 
> > > > > > > > > If you think the maybe_fold code won't be reused,
> > > > > > > > > then
> > > > > > > > > it
> > > > > > > > > should
> > > > > > > > > just
> > > > > > > > > be
> > > > > > > > > hoisted up and the 'goto' removed.
> > > > > > > > 
> > > > > > > > This actually depends on how the support for cast
> > > > > > > > between
> > > > > > > > integers
> > > > > > > > and 
> > > > > > > > pointers will be implemented (see below).
> > > > > > > > If we will support truncating pointers (does that even
> > > > > > > > make
> > > > > > > > sense?
> > > > > > > > and
> > > > > > > > I 
> > > > > > > > guess we cannot extend a pointer unless we add the
> > > > > > > > support
> > > > > > > > for 
> > > > > > > > uint128_t), that label will be reused for that case.
> > > > > > > > Otherwise, it might not be reused.
> > > > > > > > 
> > > > > > > > So, please tell me which option to choose and I'll
> > > > > > > > update
> > > > > > > > my
> > > > > > > > patch.
> > > > > > > 
> > > > > > > FWIW I don't think we'll want to support truncating or
> > > > > > > extending
> > > > > > > pointers.
> > > > > > 
> > > > > > Ok, but do you think we'll want to support casts between
> > > > > > integers
> > > > > > and
> > > > > > pointers?
> > > > > 
> > > > > Yes, though we probably want to reject truncating a pointer
> > > > > into
> > > > > a
> > > > > smaller integer type.
> > > > > 
> > > > > > I opened an issue about this
> > > > > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and
> > > > > > would
> > > > > > be
> > > > > > willing to do a patch for it eventually.
> > > > > > 
> > > > > > > > 
> > > > > > > > > On the other hand, if the maybe_fold code might be
> > > > > > > > > reused
> > > > > > > > > for
> > > > > > > > > some
> > > > > > > > > other
> > > > > > > > > case, then I suppose I would have the case end with
> > > > > > > > > 'break'
> > > > > > > > > and
> > > > > > > > > then
> > > > > > > > > have this code outside the switch.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > In another message, you wrote:
> > > > > > > > > 
> > > > > > > > > Antoni> For your question, the current code already
> > > > > > > > > works
> > > > > > > > > with
> > > > > > > > > boolean and
> > > > > > > > > Antoni> reals and casts between integers and pointers
> > > > > > > > > is
> > > > > > > > > currently
> > > > > > > > > not
> > > > > > > > > Antoni> supported.
> > > > > > > > > 
> > > > > > > > > I am curious why this wasn't supported.  It seems
> > > > > > > > > like
> > > > > > > > > something
> > > > > > > > > that
> > > > > > > > > one might want to do.
> > > > > > > > 
> > > > > > > > I have no idea as this is my first contribution to gcc.
> > > > > > > > But this would be indeed very useful and I opened an
> > > > > > > > issue
> > > > > > > > about
> > > > > > > > this: 
> > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > > > > > > 
> > > > > > > > > thanks,
> > > > > > > > > Tom
> > > > > > > > 
> > > > > > > > Thanks!
> > > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > 
> > > 
> > > 
> > 
> 
> 


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

From 18b9b05268c53368631937e9bdf3086aab8883dd 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                       | 33 ++++++----
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
 gcc/testsuite/jit.dg/test-cast.c             | 66 ++++++++++++++++++++
 3 files changed, 98 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 c6136301243..d26f08bfc1b 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -62,22 +62,33 @@ 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:
+      return fold (convert_to_integer (dst_type, expr));
+
+    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;
+    }
 }
 
 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.31.1

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

[David Malcolm]

On Fri, 2021-06-18 at 16:42 -0400, Antoni Boucher wrote:
> I'm sending the patch once again for review.
> 
> As it's the first time I'll land a patch, I'm not sure what needs to be
> done when it's approved.
> Do I just commit it to the master branch directly?

Commit (and push), yes, but...

Please ensure the subject line of the commit matches our policy:

"libgccjit: Handle truncation and extension for casts [PR95498]"

is a good subject, whereas:

"This patch handles truncation and extension for casts in jit."

is not.

We have some hooks that will only accept pushes if the commit message
has a correctly formatted ChangeLog.  The hooks ought to also add
notification comments to bugzilla for any bugs mentioned in the commit
message.

See https://gcc.gnu.org/gitwrite.html#checkin


FWIW in my own workflow I have a writable working copy that I keep for
just doing pushes; 
I do a
  git pull
verify the build, then
  git am FOO.patch --ignore-date
to apply the patch that was tested and approved, then do a last test
with that, then
  git push

That way I only push the patches that I've tested and have been
approved, and the other ones are in an entirely separate working copy.
This may be overkill though.

I'm also on #gcc IRC right now on OFTC (dmalcolm) if you run into
issues.

Dave

> 
> Thanks!
> 
> Le vendredi 11 juin 2021 à 13:49 -0400, David Malcolm a écrit :
> > On Thu, 2021-05-27 at 21:22 -0400, Antoni Boucher wrote:
> > > Here's the patch with the condition removed.
> > > I believe everything is now fixed.
> > > Thanks!
> > 
> > Thanks; this looks good to me.  Is this the latest version of the
> > patch; would you like me to apply it?
> > 
> > Dave
> > 
> > > 
> > > Le jeudi 27 mai 2021 à 18:21 -0400, David Malcolm a écrit :
> > > > On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> > > > > I updated the patch according to the comments by Tom Tromey.
> > > > > 
> > > > > There's one question left about your question regarding
> > > > > C_MAYBE_CONST_EXPR, David:
> > > > > 
> > > > > I am not sure if we can get a C_MAYBE_CONST_EXPR from
> > > > > libgccjit,
> > > > > and
> > > > > it
> > > > > indeed seems like it's only created in c-family.
> > > > > However, we do use it in libgccjit here:
> > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > > 
> > > > > I tried removing the condition `if (TREE_CODE (t_ret) !=
> > > > > C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.
> > > > > 
> > > > > That code was copied from here:
> > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> > > > > and might not be needed in libgccjit.
> > > > > 
> > > > > Should I just remove the condition, then?
> > > > 
> > > > I think so.
> > > > 
> > > > Thanks
> > > > Dave
> > > > 
> > > > > 
> > > > > Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > > > > > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > > > > > Thanks for your answer.
> > > > > > > 
> > > > > > > See my answers below:
> > > > > > > 
> > > > > > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > > > > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via
> > > > > > > > Gcc-
> > > > > > > > patches
> > > > > > > > wrote:
> > > > > > > > > Hi.
> > > > > > > > > Thanks for your feedback!
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Sorry about the delay in responding.
> > > > > > > > 
> > > > > > > > In the past I was hesitant about adding more cast support
> > > > > > > > to
> > > > > > > > libgccjit
> > > > > > > > since I felt that the user could always just create a
> > > > > > > > union
> > > > > > > > to
> > > > > > > > do
> > > > > > > > the
> > > > > > > > cast.  Then I tried actually using the libgccjit API to
> > > > > > > > do
> > > > > > > > this,
> > > > > > > > and
> > > > > > > > realized how much work it adds, so I now think we do want
> > > > > > > > to
> > > > > > > > support
> > > > > > > > casting more types.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > See answers below:
> > > > > > > > > 
> > > > > > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey
> > > > > > > > > wrote:
> > > > > > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches
> > > > > > > > > > > > > > > <   
> > > > > > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > > > > > 
> > > > > > > > > > Antoni> gcc/jit/
> > > > > > > > > > Antoni>         PR target/95498
> > > > > > > > > > Antoni>         * jit-playback.c: Add support to
> > > > > > > > > > handle
> > > > > > > > > > truncation
> > > > > > > > > > and extension
> > > > > > > > > > Antoni>         in the convert function.
> > > > > > > > > > 
> > > > > > > > > > Antoni> +  switch (dst_code)
> > > > > > > > > > Antoni> +    {
> > > > > > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > > > > > Antoni> +      t_ret = convert_to_integer (dst_type,
> > > > > > > > > > expr);
> > > > > > > > > > Antoni> +      goto maybe_fold;
> > > > > > > > > > Antoni> +
> > > > > > > > > > Antoni> +    default:
> > > > > > > > > > Antoni> +      gcc_assert
> > > > > > > > > > (gcc::jit::active_playback_ctxt);
> > > > > > > > > > Antoni> +      gcc::jit::active_playback_ctxt-
> > > > > > > > > > > add_error
> > > > > > > > > > (NULL,
> > > > > > > > > > "unhandled conversion");
> > > > > > > > > > Antoni> +      fprintf (stderr, "input
> > > > > > > > > > expression:\n");
> > > > > > > > > > Antoni> +      debug_tree (expr);
> > > > > > > > > > Antoni> +      fprintf (stderr, "requested type:\n");
> > > > > > > > > > Antoni> +      debug_tree (dst_type);
> > > > > > > > > > Antoni> +      return error_mark_node;
> > > > > > > > > > Antoni> +
> > > > > > > > > > Antoni> +    maybe_fold:
> > > > > > > > > > Antoni> +      if (TREE_CODE (t_ret) !=
> > > > > > > > > > C_MAYBE_CONST_EXPR)
> > > > > > > > 
> > > > > > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That
> > > > > > > > tree
> > > > > > > > code
> > > > > > > > is
> > > > > > > > defined in c-family/c-common.def; how can nodes of that
> > > > > > > > kind
> > > > > > > > be
> > > > > > > > created
> > > > > > > > outside of the c-family?
> > > > > > > 
> > > > > > > I am not sure, but that seems like it's only created in c-
> > > > > > > family
> > > > > > > indeed.
> > > > > > > However, we do use it in libgccjit here:
> > > > > > > 
> > > > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > > > > 
> > > > > > > > 
> > > > > > > > > > Antoni> +       t_ret = fold (t_ret);
> > > > > > > > > > Antoni> +      return t_ret;
> > > > > > > > > > 
> > > > > > > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > > > > > > especially
> > > > > > > > > > inside
> > > > > > > > > > a switch like this.
> > > > > > > > > > 
> > > > > > > > > > If you think the maybe_fold code won't be reused,
> > > > > > > > > > then
> > > > > > > > > > it
> > > > > > > > > > should
> > > > > > > > > > just
> > > > > > > > > > be
> > > > > > > > > > hoisted up and the 'goto' removed.
> > > > > > > > > 
> > > > > > > > > This actually depends on how the support for cast
> > > > > > > > > between
> > > > > > > > > integers
> > > > > > > > > and 
> > > > > > > > > pointers will be implemented (see below).
> > > > > > > > > If we will support truncating pointers (does that even
> > > > > > > > > make
> > > > > > > > > sense?
> > > > > > > > > and
> > > > > > > > > I 
> > > > > > > > > guess we cannot extend a pointer unless we add the
> > > > > > > > > support
> > > > > > > > > for 
> > > > > > > > > uint128_t), that label will be reused for that case.
> > > > > > > > > Otherwise, it might not be reused.
> > > > > > > > > 
> > > > > > > > > So, please tell me which option to choose and I'll
> > > > > > > > > update
> > > > > > > > > my
> > > > > > > > > patch.
> > > > > > > > 
> > > > > > > > FWIW I don't think we'll want to support truncating or
> > > > > > > > extending
> > > > > > > > pointers.
> > > > > > > 
> > > > > > > Ok, but do you think we'll want to support casts between
> > > > > > > integers
> > > > > > > and
> > > > > > > pointers?
> > > > > > 
> > > > > > Yes, though we probably want to reject truncating a pointer
> > > > > > into
> > > > > > a
> > > > > > smaller integer type.
> > > > > > 
> > > > > > > I opened an issue about this
> > > > > > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and
> > > > > > > would
> > > > > > > be
> > > > > > > willing to do a patch for it eventually.
> > > > > > > 
> > > > > > > > > 
> > > > > > > > > > On the other hand, if the maybe_fold code might be
> > > > > > > > > > reused
> > > > > > > > > > for
> > > > > > > > > > some
> > > > > > > > > > other
> > > > > > > > > > case, then I suppose I would have the case end with
> > > > > > > > > > 'break'
> > > > > > > > > > and
> > > > > > > > > > then
> > > > > > > > > > have this code outside the switch.
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > In another message, you wrote:
> > > > > > > > > > 
> > > > > > > > > > Antoni> For your question, the current code already
> > > > > > > > > > works
> > > > > > > > > > with
> > > > > > > > > > boolean and
> > > > > > > > > > Antoni> reals and casts between integers and pointers
> > > > > > > > > > is
> > > > > > > > > > currently
> > > > > > > > > > not
> > > > > > > > > > Antoni> supported.
> > > > > > > > > > 
> > > > > > > > > > I am curious why this wasn't supported.  It seems
> > > > > > > > > > like
> > > > > > > > > > something
> > > > > > > > > > that
> > > > > > > > > > one might want to do.
> > > > > > > > > 
> > > > > > > > > I have no idea as this is my first contribution to gcc.
> > > > > > > > > But this would be indeed very useful and I opened an
> > > > > > > > > issue
> > > > > > > > > about
> > > > > > > > > this: 
> > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > > > > > > > 
> > > > > > > > > > thanks,
> > > > > > > > > > Tom
> > > > > > > > > 
> > > > > > > > > Thanks!
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > 
> > 
> 

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

[Antoni Boucher]

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

Ok.
Here's the patch with the updated subject and with format fixed.

Le vendredi 18 juin 2021 à 16:54 -0400, David Malcolm a écrit :
> On Fri, 2021-06-18 at 16:42 -0400, Antoni Boucher wrote:
> > I'm sending the patch once again for review.
> > 
> > As it's the first time I'll land a patch, I'm not sure what needs
> > to be
> > done when it's approved.
> > Do I just commit it to the master branch directly?
> 
> Commit (and push), yes, but...
> 
> Please ensure the subject line of the commit matches our policy:
> 
> "libgccjit: Handle truncation and extension for casts [PR95498]"
> 
> is a good subject, whereas:
> 
> "This patch handles truncation and extension for casts in jit."
> 
> is not.
> 
> We have some hooks that will only accept pushes if the commit message
> has a correctly formatted ChangeLog.  The hooks ought to also add
> notification comments to bugzilla for any bugs mentioned in the
> commit
> message.
> 
> See https://gcc.gnu.org/gitwrite.html#checkin
> 
> 
> FWIW in my own workflow I have a writable working copy that I keep
> for
> just doing pushes; 
> I do a
>   git pull
> verify the build, then
>   git am FOO.patch --ignore-date
> to apply the patch that was tested and approved, then do a last test
> with that, then
>   git push
> 
> That way I only push the patches that I've tested and have been
> approved, and the other ones are in an entirely separate working
> copy.
> This may be overkill though.
> 
> I'm also on #gcc IRC right now on OFTC (dmalcolm) if you run into
> issues.
> 
> Dave
> 
> > 
> > Thanks!
> > 
> > Le vendredi 11 juin 2021 à 13:49 -0400, David Malcolm a écrit :
> > > On Thu, 2021-05-27 at 21:22 -0400, Antoni Boucher wrote:
> > > > Here's the patch with the condition removed.
> > > > I believe everything is now fixed.
> > > > Thanks!
> > > 
> > > Thanks; this looks good to me.  Is this the latest version of the
> > > patch; would you like me to apply it?
> > > 
> > > Dave
> > > 
> > > > 
> > > > Le jeudi 27 mai 2021 à 18:21 -0400, David Malcolm a écrit :
> > > > > On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> > > > > > I updated the patch according to the comments by Tom
> > > > > > Tromey.
> > > > > > 
> > > > > > There's one question left about your question regarding
> > > > > > C_MAYBE_CONST_EXPR, David:
> > > > > > 
> > > > > > I am not sure if we can get a C_MAYBE_CONST_EXPR from
> > > > > > libgccjit,
> > > > > > and
> > > > > > it
> > > > > > indeed seems like it's only created in c-family.
> > > > > > However, we do use it in libgccjit here:
> > > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > > > 
> > > > > > I tried removing the condition `if (TREE_CODE (t_ret) !=
> > > > > > C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still
> > > > > > pass.
> > > > > > 
> > > > > > That code was copied from here:
> > > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> > > > > > and might not be needed in libgccjit.
> > > > > > 
> > > > > > Should I just remove the condition, then?
> > > > > 
> > > > > I think so.
> > > > > 
> > > > > Thanks
> > > > > Dave
> > > > > 
> > > > > > 
> > > > > > Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > > > > > > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > > > > > > Thanks for your answer.
> > > > > > > > 
> > > > > > > > See my answers below:
> > > > > > > > 
> > > > > > > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a
> > > > > > > > écrit :
> > > > > > > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via
> > > > > > > > > Gcc-
> > > > > > > > > patches
> > > > > > > > > wrote:
> > > > > > > > > > Hi.
> > > > > > > > > > Thanks for your feedback!
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Sorry about the delay in responding.
> > > > > > > > > 
> > > > > > > > > In the past I was hesitant about adding more cast
> > > > > > > > > support
> > > > > > > > > to
> > > > > > > > > libgccjit
> > > > > > > > > since I felt that the user could always just create a
> > > > > > > > > union
> > > > > > > > > to
> > > > > > > > > do
> > > > > > > > > the
> > > > > > > > > cast.  Then I tried actually using the libgccjit API
> > > > > > > > > to
> > > > > > > > > do
> > > > > > > > > this,
> > > > > > > > > and
> > > > > > > > > realized how much work it adds, so I now think we do
> > > > > > > > > want
> > > > > > > > > to
> > > > > > > > > support
> > > > > > > > > casting more types.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > See answers below:
> > > > > > > > > > 
> > > > > > > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom
> > > > > > > > > > Tromey
> > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-
> > > > > > > > > > > > > > > > patches
> > > > > > > > > > > > > > > > <   
> > > > > > > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > > > > > > 
> > > > > > > > > > > Antoni> gcc/jit/
> > > > > > > > > > > Antoni>         PR target/95498
> > > > > > > > > > > Antoni>         * jit-playback.c: Add support to
> > > > > > > > > > > handle
> > > > > > > > > > > truncation
> > > > > > > > > > > and extension
> > > > > > > > > > > Antoni>         in the convert function.
> > > > > > > > > > > 
> > > > > > > > > > > Antoni> +  switch (dst_code)
> > > > > > > > > > > Antoni> +    {
> > > > > > > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > > > > > > Antoni> +      t_ret = convert_to_integer
> > > > > > > > > > > (dst_type,
> > > > > > > > > > > expr);
> > > > > > > > > > > Antoni> +      goto maybe_fold;
> > > > > > > > > > > Antoni> +
> > > > > > > > > > > Antoni> +    default:
> > > > > > > > > > > Antoni> +      gcc_assert
> > > > > > > > > > > (gcc::jit::active_playback_ctxt);
> > > > > > > > > > > Antoni> +      gcc::jit::active_playback_ctxt-
> > > > > > > > > > > > add_error
> > > > > > > > > > > (NULL,
> > > > > > > > > > > "unhandled conversion");
> > > > > > > > > > > Antoni> +      fprintf (stderr, "input
> > > > > > > > > > > expression:\n");
> > > > > > > > > > > Antoni> +      debug_tree (expr);
> > > > > > > > > > > Antoni> +      fprintf (stderr, "requested
> > > > > > > > > > > type:\n");
> > > > > > > > > > > Antoni> +      debug_tree (dst_type);
> > > > > > > > > > > Antoni> +      return error_mark_node;
> > > > > > > > > > > Antoni> +
> > > > > > > > > > > Antoni> +    maybe_fold:
> > > > > > > > > > > Antoni> +      if (TREE_CODE (t_ret) !=
> > > > > > > > > > > C_MAYBE_CONST_EXPR)
> > > > > > > > > 
> > > > > > > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That
> > > > > > > > > tree
> > > > > > > > > code
> > > > > > > > > is
> > > > > > > > > defined in c-family/c-common.def; how can nodes of
> > > > > > > > > that
> > > > > > > > > kind
> > > > > > > > > be
> > > > > > > > > created
> > > > > > > > > outside of the c-family?
> > > > > > > > 
> > > > > > > > I am not sure, but that seems like it's only created in
> > > > > > > > c-
> > > > > > > > family
> > > > > > > > indeed.
> > > > > > > > However, we do use it in libgccjit here:
> > > > > > > > 
> > > > > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > > Antoni> +       t_ret = fold (t_ret);
> > > > > > > > > > > Antoni> +      return t_ret;
> > > > > > > > > > > 
> > > > > > > > > > > It seems weird to have a single 'goto' to
> > > > > > > > > > > maybe_fold,
> > > > > > > > > > > especially
> > > > > > > > > > > inside
> > > > > > > > > > > a switch like this.
> > > > > > > > > > > 
> > > > > > > > > > > If you think the maybe_fold code won't be reused,
> > > > > > > > > > > then
> > > > > > > > > > > it
> > > > > > > > > > > should
> > > > > > > > > > > just
> > > > > > > > > > > be
> > > > > > > > > > > hoisted up and the 'goto' removed.
> > > > > > > > > > 
> > > > > > > > > > This actually depends on how the support for cast
> > > > > > > > > > between
> > > > > > > > > > integers
> > > > > > > > > > and 
> > > > > > > > > > pointers will be implemented (see below).
> > > > > > > > > > If we will support truncating pointers (does that
> > > > > > > > > > even
> > > > > > > > > > make
> > > > > > > > > > sense?
> > > > > > > > > > and
> > > > > > > > > > I 
> > > > > > > > > > guess we cannot extend a pointer unless we add the
> > > > > > > > > > support
> > > > > > > > > > for 
> > > > > > > > > > uint128_t), that label will be reused for that
> > > > > > > > > > case.
> > > > > > > > > > Otherwise, it might not be reused.
> > > > > > > > > > 
> > > > > > > > > > So, please tell me which option to choose and I'll
> > > > > > > > > > update
> > > > > > > > > > my
> > > > > > > > > > patch.
> > > > > > > > > 
> > > > > > > > > FWIW I don't think we'll want to support truncating
> > > > > > > > > or
> > > > > > > > > extending
> > > > > > > > > pointers.
> > > > > > > > 
> > > > > > > > Ok, but do you think we'll want to support casts
> > > > > > > > between
> > > > > > > > integers
> > > > > > > > and
> > > > > > > > pointers?
> > > > > > > 
> > > > > > > Yes, though we probably want to reject truncating a
> > > > > > > pointer
> > > > > > > into
> > > > > > > a
> > > > > > > smaller integer type.
> > > > > > > 
> > > > > > > > I opened an issue about this
> > > > > > > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438)
> > > > > > > > and
> > > > > > > > would
> > > > > > > > be
> > > > > > > > willing to do a patch for it eventually.
> > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > On the other hand, if the maybe_fold code might
> > > > > > > > > > > be
> > > > > > > > > > > reused
> > > > > > > > > > > for
> > > > > > > > > > > some
> > > > > > > > > > > other
> > > > > > > > > > > case, then I suppose I would have the case end
> > > > > > > > > > > with
> > > > > > > > > > > 'break'
> > > > > > > > > > > and
> > > > > > > > > > > then
> > > > > > > > > > > have this code outside the switch.
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > In another message, you wrote:
> > > > > > > > > > > 
> > > > > > > > > > > Antoni> For your question, the current code
> > > > > > > > > > > already
> > > > > > > > > > > works
> > > > > > > > > > > with
> > > > > > > > > > > boolean and
> > > > > > > > > > > Antoni> reals and casts between integers and
> > > > > > > > > > > pointers
> > > > > > > > > > > is
> > > > > > > > > > > currently
> > > > > > > > > > > not
> > > > > > > > > > > Antoni> supported.
> > > > > > > > > > > 
> > > > > > > > > > > I am curious why this wasn't supported.  It seems
> > > > > > > > > > > like
> > > > > > > > > > > something
> > > > > > > > > > > that
> > > > > > > > > > > one might want to do.
> > > > > > > > > > 
> > > > > > > > > > I have no idea as this is my first contribution to
> > > > > > > > > > gcc.
> > > > > > > > > > But this would be indeed very useful and I opened
> > > > > > > > > > an
> > > > > > > > > > issue
> > > > > > > > > > about
> > > > > > > > > > this: 
> > > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438
> > > > > > > > > > 
> > > > > > > > > > > thanks,
> > > > > > > > > > > Tom
> > > > > > > > > > 
> > > > > > > > > > Thanks!
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > 
> > > 
> > > 
> > 
> 
> 


[-- Attachment #2: 0001-libgccjit-Handle-truncation-and-extension-for-casts-.patch --]
[-- Type: text/x-patch, Size: 5262 bytes --]

From c6235c35364886e95376702240ad217106969b58 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Sun, 5 Jul 2020 19:07:30 -0400
Subject: [PATCH] libgccjit: Handle truncation and extension for casts
 [PR95498]

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                       | 33 ++++++----
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
 gcc/testsuite/jit.dg/test-cast.c             | 66 ++++++++++++++++++++
 3 files changed, 98 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 c6136301243..d26f08bfc1b 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -62,22 +62,33 @@ 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:
+      return fold (convert_to_integer (dst_type, expr));
+
+    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;
+    }
 }
 
 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.32.0

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

[Bernhard Reutner-Fischer]

On 18 June 2021 23:11:58 CEST, Antoni Boucher via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>Ok.
>Here's the patch with the updated subject and with format fixed.

+  if (t_ret)
+      return t_ret;
+  enum tree_code dst_code = TREE_CODE (dst_type);
+  switch (dst_code)
+    {

The enum is redundant.
/enum tree_code/s/enum //
thanks,

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

[Antoni Boucher]

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

I updated the patch to fix this.
I'm sending the patch once again for review.
Thanks.

Le samedi 19 juin 2021 à 11:08 +0200, Bernhard Reutner-Fischer a
écrit :
> On 18 June 2021 23:11:58 CEST, Antoni Boucher via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> > Ok.
> > Here's the patch with the updated subject and with format fixed.
> 
> +  if (t_ret)
> +      return t_ret;
> +  enum tree_code dst_code = TREE_CODE (dst_type);
> +  switch (dst_code)
> +    {
> 
> The enum is redundant.
> /enum tree_code/s/enum //
> thanks,


[-- Attachment #2: 0001-libgccjit-Handle-truncation-and-extension-for-casts-.patch --]
[-- Type: text/x-patch, Size: 5223 bytes --]

From 4647447aef34f63a94147892b9e3abc061baf184 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Sun, 5 Jul 2020 19:07:30 -0400
Subject: [PATCH] libgccjit: Handle truncation and extension for casts
 [PR95498]

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                       | 32 ++++++----
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
 gcc/testsuite/jit.dg/test-cast.c             | 66 ++++++++++++++++++++
 3 files changed, 97 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 c6136301243..79ac525e5df 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -62,22 +62,32 @@ 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;
+  switch (TREE_CODE (dst_type))
+    {
+    case INTEGER_TYPE:
+    case ENUMERAL_TYPE:
+      return fold (convert_to_integer (dst_type, expr));
+
+    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;
+    }
 }
 
 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.32.0

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

[David Malcolm]

On Tue, 2021-07-06 at 09:00 -0400, Antoni Boucher wrote:
> I updated the patch to fix this.
> I'm sending the patch once again for review.
> Thanks.

Looks good to me.

Dave

> 
> Le samedi 19 juin 2021 à 11:08 +0200, Bernhard Reutner-Fischer a
> écrit :
> > On 18 June 2021 23:11:58 CEST, Antoni Boucher via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > > Ok.
> > > Here's the patch with the updated subject and with format fixed.
> > 
> > +  if (t_ret)
> > +      return t_ret;
> > +  enum tree_code dst_code = TREE_CODE (dst_type);
> > +  switch (dst_code)
> > +    {
> > 
> > The enum is redundant.
> > /enum tree_code/s/enum //
> > thanks,
> 

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

[Antoni Boucher]

Is this the approval for write or are we not there yet?

Le jeudi 08 juillet 2021 à 16:44 -0400, David Malcolm a écrit :
> On Tue, 2021-07-06 at 09:00 -0400, Antoni Boucher wrote:
> > I updated the patch to fix this.
> > I'm sending the patch once again for review.
> > Thanks.
> 
> Looks good to me.
> 
> Dave
> 
> > 
> > Le samedi 19 juin 2021 à 11:08 +0200, Bernhard Reutner-Fischer a
> > écrit :
> > > On 18 June 2021 23:11:58 CEST, Antoni Boucher via Gcc-patches
> > > <gcc-patches@gcc.gnu.org> wrote:
> > > > Ok.
> > > > Here's the patch with the updated subject and with format
> > > > fixed.
> > > 
> > > +  if (t_ret)
> > > +      return t_ret;
> > > +  enum tree_code dst_code = TREE_CODE (dst_type);
> > > +  switch (dst_code)
> > > +    {
> > > 
> > > The enum is redundant.
> > > /enum tree_code/s/enum //
> > > thanks,
> > 
> 
> 

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

[David Malcolm]

On Thu, 2021-07-08 at 17:28 -0400, Antoni Boucher wrote:
> Is this the approval for write or are we not there yet?

Assuming it passes the regression tests, yes - approved for trunk.

Dave

> 
> Le jeudi 08 juillet 2021 à 16:44 -0400, David Malcolm a écrit :
> > On Tue, 2021-07-06 at 09:00 -0400, Antoni Boucher wrote:
> > > I updated the patch to fix this.
> > > I'm sending the patch once again for review.
> > > Thanks.
> > 
> > Looks good to me.
> > 
> > Dave
> > 
> > > 
> > > Le samedi 19 juin 2021 à 11:08 +0200, Bernhard Reutner-Fischer a
> > > écrit :
> > > > On 18 June 2021 23:11:58 CEST, Antoni Boucher via Gcc-patches
> > > > <gcc-patches@gcc.gnu.org> wrote:
> > > > > Ok.
> > > > > Here's the patch with the updated subject and with format
> > > > > fixed.
> > > > 
> > > > +  if (t_ret)
> > > > +      return t_ret;
> > > > +  enum tree_code dst_code = TREE_CODE (dst_type);
> > > > +  switch (dst_code)
> > > > +    {
> > > > 
> > > > The enum is redundant.
> > > > /enum tree_code/s/enum //
> > > > thanks,
> > > 
> > 
> > 
> 
>