public inbox for jit@gcc.gnu.org
 help / color / mirror / Atom feed
From: Basile Starynkevitch <basile@starynkevitch.net>
To: David Malcolm <dmalcolm@redhat.com>
Cc: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org
Subject: PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
Date: Thu, 01 Jan 2015 00:00:00 -0000	[thread overview]
Message-ID: <55A772A0.8090808@starynkevitch.net> (raw)
In-Reply-To: <1436987779.830.62.camel@surprise>

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

On 07/15/2015 21:16, David Malcolm wrote:
> Perhaps, but note that nothing in a regular gcc bootstrap uses
> libgccjit, so you *might* still have a latent linking error that shows
> up only at run time.   Running the jit testsuite is the best way to be
> sure.
>
>> And I'm testing that on
>> x86-64/Linux where the patch is almost useless.
>>
>> Thanks for your other comments. I'm trying to understand them and I am
>> working on that.
>>
>> Cheers
>>

Here (attached gcc-jitlonglong-r225860.diff)
is an improved version of my patch against trunk r225860.
Thanks to David Malcom for the kind help.

### gcc/jit/ ChangeLog entry

2015-07-16  Basile Starynkevitch  <basile@starynkevitch.net>

     * jit-playback.c: Mention that it is in C++.
     (new_rvalue_from_const <long>): New.

     * jit-recording.c: Mention that it is in C++.
     (recording::memento_of_new_rvalue_from_const <long long>): New
     instanciated template.
     (memento_of_new_rvalue_from_const <long long>::make_debug_string):
     New specialized function.
     (memento_of_new_rvalue_from_const <long long>::get_wide_int): New
     specialized function.
     (recording::memento_of_new_rvalue_from_const <long
     long>::write_reproducer): Likewise.

     * libgccjit.c: Mention that it is in C++.
     (gcc_jit_context_new_rvalue_from_long_long): New function.
     (gcc_jit_context_new_rvalue_from_int32): New function.
     (gcc_jit_context_new_rvalue_from_int64): New function.
     (gcc_jit_context_new_rvalue_from_intptr): New function.

     * libgccjit.h: #include <stdint.h>
     (gcc_jit_context_new_rvalue_from_long_long): New declaration.
     In the declarations of the functions below, a short comment
     explains that they are convenience functions.
     (gcc_jit_context_new_rvalue_from_int32): New declaration.
     (gcc_jit_context_new_rvalue_from_int64): New declaration.
     (gcc_jit_context_new_rvalue_from_intptr): New declaration.

     * libgccjit.map: Add LIBGCCJIT_ABI_4 for new functions
     e.g. gcc_jit_context_new_rvalue_from_long_long, ....


## gcc/testsuite/ChangeLog entry
     * test-constants.c (make_test_of_long_long_constant): New function.
     (make_tests_of_long_long_constants): New.
     (verify_long_long_constants): New.
     (create_code): Call make_tests_of_long_long_constants.
     (verify_code): Call verify_long_long_constants.


I have mixed feelings about adding the 
gcc_jit_context_new_rvalue_from_int32 
gcc_jit_context_new_rvalue_from_int64 & 
gcc_jit_context_new_rvalue_from_intptr functions.
On one hand, their name is very suggestive, and most programmers know 
about <stdint.h>. I should confess that I discovered only recently that 
long long is guaranteed by C99 standard to be at least 64 bits (I 
thought that the standard just required that long long is at least as 
big as long).
On the other hand, we are adding more functions to the ABI, and indeed 
the gcc_jit_context_new_rvalue_from_long_long is in principle enough. 
Perhaps we should simply document that for int32_t, int64_t, intptr_t 
types, the GCCJIT user should test the sizeof intptr_t and call the 
appropriate function?

BTW some bytecodes or VMs (in particular the JVM) are hardcoding the 
size of some integers, so dealing explicitly with int32_t & int64_t 
definitely makes sense.

The patch is passing the test that I have added, on Debian/x86-64.

Comments are welcome, including perhaps an Ok for trunk...

Regards.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***


[-- Attachment #2: gcc-jitlonglong-r225860.diff --]
[-- Type: text/x-patch, Size: 13373 bytes --]

Index: gcc/jit/jit-playback.c
===================================================================
--- gcc/jit/jit-playback.c	(revision 225860)
+++ gcc/jit/jit-playback.c	(working copy)
@@ -1,4 +1,5 @@
-/* Internals of libgccjit: classes for playing back recorded API calls.
+/* This jit-playback.c file is in  -*- C++ -*- ...
+   Internals of libgccjit: classes for playing back recorded API calls.
    Copyright (C) 2013-2015 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
 
@@ -573,6 +574,30 @@ new_rvalue_from_const <long> (type *type,
     }
 }
 
+ 
+/* Specialization of making an rvalue from a const, for host <long long>.  */
+
+template <>
+rvalue *
+context::
+new_rvalue_from_const <long long> (type *type,
+				   long long value)
+{
+  // FIXME: type-checking, or coercion?
+  tree inner_type = type->as_tree ();
+  if (INTEGRAL_TYPE_P (inner_type))
+    {
+      tree inner = build_int_cst (inner_type, value);
+      return new rvalue (this, inner);
+    }
+  else
+    {
+      REAL_VALUE_TYPE real_value;
+      real_from_integer (&real_value, VOIDmode, value, SIGNED);
+      tree inner = build_real (inner_type, real_value);
+      return new rvalue (this, inner);
+    }
+}
 /* Specialization of making an rvalue from a const, for host <double>.  */
 
 template <>
Index: gcc/jit/jit-recording.c
===================================================================
--- gcc/jit/jit-recording.c	(revision 225860)
+++ gcc/jit/jit-recording.c	(working copy)
@@ -1,4 +1,5 @@
-/* Internals of libgccjit: classes for recording calls made to the JIT API.
+/* This jit-recording.c file is in  -*- C++ -*- ...
+   Internals of libgccjit: classes for recording calls made to the JIT API.
    Copyright (C) 2013-2015 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
 
@@ -4072,6 +4073,7 @@ recording::global::write_reproducer (reproducer &r
 /* Explicit specialization of the various mementos we're interested in.  */
 template class recording::memento_of_new_rvalue_from_const <int>;
 template class recording::memento_of_new_rvalue_from_const <long>;
+template class recording::memento_of_new_rvalue_from_const <long long>;
 template class recording::memento_of_new_rvalue_from_const <double>;
 template class recording::memento_of_new_rvalue_from_const <void *>;
 
@@ -4161,6 +4163,22 @@ memento_of_new_rvalue_from_const <long>::make_debu
 			      m_value);
 }
 
+
+/* The make_debug_string specialization for <long long>, rendering it as
+     (TARGET_TYPE)LITERAL
+   e.g.
+     "(long long)42".  */
+
+template <>
+string *
+memento_of_new_rvalue_from_const <long long>::make_debug_string ()
+{
+  return string::from_printf (m_ctxt,
+			      "(%s)%lli",
+			      m_type->get_debug_string (),
+			      m_value);
+}
+
 /* The get_wide_int specialization for <long>.  */
 
 template <>
@@ -4171,6 +4189,16 @@ memento_of_new_rvalue_from_const <long>::get_wide_
   return true;
 }
 
+/* The get_wide_int specialization for <long long>.  */
+
+template <>
+bool
+memento_of_new_rvalue_from_const <long long>::get_wide_int (wide_int *out) const
+{
+  *out = wi::shwi (m_value, sizeof (m_value) * 8);
+  return true;
+}
+
 /* The write_reproducer specialization for <long>.  */
 
 template <>
@@ -4209,6 +4237,43 @@ recording::memento_of_new_rvalue_from_const <long>
 	   m_value);
 	   }
 
+ 
+/* The write_reproducer specialization for <long long>.  */
+
+template <>
+void
+recording::memento_of_new_rvalue_from_const <long long>::write_reproducer (reproducer &r)
+{
+  const char *id = r.make_identifier (this, "rvalue");
+
+  /* We have to special-case LONG_LONG_MIN like in the previous specialization
+     and hence we'd get:
+	error: integer constant is so large that it is unsigned [-Werror]
+	Workaround this by writing (LONG_LONG_MIN + 1) - 1.  */
+  if (m_value == LONG_LONG_MIN)
+    {
+      r.write ("  gcc_jit_rvalue *%s =\n"
+	       "    gcc_jit_context_new_rvalue_from_long_long (%s, /* gcc_jit_context *ctxt */\n"
+	       "                                          %s, /* gcc_jit_type *numeric_type */\n"
+	       "                                          %lldLL - 1); /* long long value */\n",
+	       id,
+	       r.get_identifier (get_context ()),
+	       r.get_identifier_as_type (m_type),
+	       m_value + 1);;
+      return;
+    }
+
+  r.write ("  gcc_jit_rvalue *%s =\n"
+	   "    gcc_jit_context_new_rvalue_from_long_long (%s, /* gcc_jit_context *ctxt */\n"
+	   "                                          %s, /* gcc_jit_type *numeric_type */\n"
+	   "                                          %lldLL); /* long long value */\n",
+	   id,
+	   r.get_identifier (get_context ()),
+	   r.get_identifier_as_type (m_type),
+	   m_value);
+	   }
+
+ 
 /* The make_debug_string specialization for <double>, rendering it as
      (TARGET_TYPE)LITERAL
    e.g.
Index: gcc/jit/libgccjit.c
===================================================================
--- gcc/jit/libgccjit.c	(revision 225860)
+++ gcc/jit/libgccjit.c	(working copy)
@@ -1,4 +1,5 @@
-/* Implementation of the C API; all wrappers into the internal C++ API
+/* This libgccjit.c file is in  -*- C++ -*- ...
+   Implementation of the C API; all wrappers into the internal C++ API
    Copyright (C) 2013-2015 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
 
@@ -1154,6 +1155,77 @@ gcc_jit_context_new_rvalue_from_long (gcc_jit_cont
 	  ->new_rvalue_from_const <long> (numeric_type, value));
 }
 
+/* Public entrypoint.  See description in libgccjit.h.  */
+
+gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
+				     gcc_jit_type *numeric_type,
+				     long long value)
+{
+  RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
+  JIT_LOG_FUNC (ctxt->get_logger ());
+  RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
+
+  return ((gcc_jit_rvalue *)ctxt
+	  ->new_rvalue_from_const <long long> (numeric_type, value));
+}
+
+
+/* Public entrypoint.  See description in libgccjit.h.  */
+
+gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int32 (gcc_jit_context *ctxt,
+				     gcc_jit_type *numeric_type,
+				     int32_t value)
+{
+  RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
+  JIT_LOG_FUNC (ctxt->get_logger ());
+  RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
+
+  return ((gcc_jit_rvalue *)ctxt
+	  ->new_rvalue_from_const <long> (numeric_type, (long) value));
+}
+
+
+/* Public entrypoint.  See description in libgccjit.h.  */
+
+gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int64 (gcc_jit_context *ctxt,
+				     gcc_jit_type *numeric_type,
+				     int64_t value)
+{
+  RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
+  JIT_LOG_FUNC (ctxt->get_logger ());
+  RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
+
+  return ((gcc_jit_rvalue *)ctxt
+	  ->new_rvalue_from_const <long long> (numeric_type, (long long) value));
+}
+
+
+/* Public entrypoint.  See description in libgccjit.h.  */
+
+gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_intptr (gcc_jit_context *ctxt,
+				     gcc_jit_type *numeric_type,
+				     intptr_t value)
+{
+  RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
+  JIT_LOG_FUNC (ctxt->get_logger ());
+  RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
+
+  if (sizeof(intptr_t) == sizeof(int))
+    return ((gcc_jit_rvalue *)ctxt
+	    ->new_rvalue_from_const <int> (numeric_type, (int) value));
+  else if (sizeof(intptr_t) == sizeof(long))
+    return ((gcc_jit_rvalue *)ctxt
+	    ->new_rvalue_from_const <long> (numeric_type, (long) value));
+  else
+    return ((gcc_jit_rvalue *)ctxt
+	    ->new_rvalue_from_const <long long> (numeric_type, (long long) value));
+}
+
+
 /* Public entrypoint.  See description in libgccjit.h.
 
    This is essentially equivalent to:
Index: gcc/jit/libgccjit.h
===================================================================
--- gcc/jit/libgccjit.h	(revision 225860)
+++ gcc/jit/libgccjit.h	(working copy)
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not see
 #define LIBGCCJIT_H
 
 #include <stdio.h>
+#include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -752,6 +753,32 @@ gcc_jit_context_new_rvalue_from_long (gcc_jit_cont
 				      long value);
 
 extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
+				      gcc_jit_type *numeric_type,
+				      long long value);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long, but takes
+   an int32_t for convenience. */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int32 (gcc_jit_context *ctxt,
+				      gcc_jit_type *numeric_type,
+				      int32_t value);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but
+   takes an int64_t for convenience. */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int64 (gcc_jit_context *ctxt,
+				      gcc_jit_type *numeric_type,
+				      int64_t value);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but
+   takes an intptr_t for convenience. */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_intptr (gcc_jit_context *ctxt,
+				      gcc_jit_type *numeric_type,
+				      intptr_t value);
+
+extern gcc_jit_rvalue *
 gcc_jit_context_zero (gcc_jit_context *ctxt,
 		      gcc_jit_type *numeric_type);
 
Index: gcc/jit/libgccjit.map
===================================================================
--- gcc/jit/libgccjit.map	(revision 225860)
+++ gcc/jit/libgccjit.map	(working copy)
@@ -128,3 +128,12 @@ LIBGCCJIT_ABI_3 {
     gcc_jit_case_as_object;
     gcc_jit_context_new_case;
 } LIBGCCJIT_ABI_2;
+
+# Add support for long long & int32 & int64 & intptr
+LIBGCCJIT_ABI_4 {
+  global:
+    gcc_jit_context_new_rvalue_from_int32;
+    gcc_jit_context_new_rvalue_from_int64;
+    gcc_jit_context_new_rvalue_from_intptr;
+    gcc_jit_context_new_rvalue_from_long_long;
+}  LIBGCCJIT_ABI_3;
\ No newline at end of file
Index: gcc/testsuite/jit.dg/test-constants.c
===================================================================
--- gcc/testsuite/jit.dg/test-constants.c	(revision 225860)
+++ gcc/testsuite/jit.dg/test-constants.c	(working copy)
@@ -173,6 +173,79 @@ verify_long_constants (gcc_jit_result *result)
 }
 
 /**********************************************************************
+ Tests of gcc_jit_context_new_rvalue_from_long_long.
+ **********************************************************************/
+
+static const char *
+make_test_of_long_long_constant (gcc_jit_context *ctxt,
+			   gcc_jit_type *type,
+			   long long value,
+			   const char *funcname)
+{
+  /* Make a test function of the form:
+       long long funcname (void)
+       {
+	  return VALUE;
+       }
+     and return a debug dump of VALUE so that
+     the caller can sanity-check the debug dump implementation.
+  */
+  gcc_jit_rvalue *rvalue =
+    gcc_jit_context_new_rvalue_from_long_long (ctxt, type, value);
+  make_test_of_constant (ctxt, type, rvalue, funcname);
+  return gcc_jit_object_get_debug_string (
+    gcc_jit_rvalue_as_object (rvalue));
+}
+
+static void
+make_tests_of_long_long_constants (gcc_jit_context *ctxt)
+{
+  gcc_jit_type *long_long_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_LONG_LONG);
+
+  CHECK_STRING_VALUE
+    (
+     make_test_of_long_long_constant (ctxt,
+				      long_long_type,
+				      0,
+				      "test_long_long_constant_0"),
+     "(long long)0");
+  make_test_of_long_long_constant (ctxt,
+				   long_long_type,
+				   LLONG_MAX,
+				   "test_long_long_constant_LLONG_MAX");
+  make_test_of_long_long_constant (ctxt,
+				   long_long_type,
+				   LLONG_MIN,
+				   "test_long_long_constant_LLONG_MIN");
+}
+
+static void
+verify_long_long_constants (gcc_jit_result *result)
+{
+  typedef long long (*test_fn) (void);
+
+  test_fn test_long_long_constant_0 =
+    (test_fn)gcc_jit_result_get_code (result,
+				      "test_long_long_constant_0");
+  CHECK_NON_NULL (test_long_long_constant_0);
+  CHECK_VALUE (test_long_long_constant_0 (), 0);
+
+  test_fn test_long_long_constant_LLONG_MAX =
+    (test_fn)gcc_jit_result_get_code (result,
+				      "test_long_long_constant_LLONG_MAX");
+  CHECK_NON_NULL (test_long_long_constant_LLONG_MAX);
+  CHECK_VALUE (test_long_long_constant_LLONG_MAX (), LLONG_MAX);
+
+  test_fn test_long_long_constant_LLONG_MIN =
+    (test_fn)gcc_jit_result_get_code (result,
+				      "test_long_long_constant_LLONG_MIN");
+  CHECK_NON_NULL (test_long_long_constant_LLONG_MIN);
+  CHECK_VALUE (test_long_long_constant_LLONG_MIN (), LLONG_MIN);
+}
+
+
+/**********************************************************************
  Tests of gcc_jit_context_new_rvalue_from_double.
  **********************************************************************/
 
@@ -323,6 +396,7 @@ create_code (gcc_jit_context *ctxt, void *user_dat
 {
   make_tests_of_int_constants (ctxt);
   make_tests_of_long_constants (ctxt);
+  make_tests_of_long_long_constants (ctxt);
   make_tests_of_double_constants (ctxt);
   make_tests_of_ptr_constants (ctxt);
 }
@@ -334,6 +408,7 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result
 
   verify_int_constants (result);
   verify_long_constants (result);
+  verify_long_long_constants (result);
   verify_double_constants (result);
   verify_ptr_constants (result);
 }

  reply	other threads:[~2015-07-16  9:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <55A6A421.8060707@starynkevitch.net>
2015-01-01  0:00 ` PATCH " David Malcolm
2015-01-01  0:00   ` Basile Starynkevitch
2015-01-01  0:00     ` David Malcolm
2015-01-01  0:00       ` Basile Starynkevitch [this message]
2015-01-01  0:00         ` PATCH (v2) " David Malcolm
2015-01-01  0:00           ` Basile Starynkevitch
2015-01-01  0:00           ` David Malcolm
2015-01-01  0:00   ` PATCH " Basile Starynkevitch
2015-01-01  0:00   ` Basile Starynkevitch
2015-01-01  0:00     ` David Malcolm
2015-01-01  0:00   ` [PATCH, committed] jit: Add guide for submitting patches to jit docs David Malcolm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55A772A0.8090808@starynkevitch.net \
    --to=basile@starynkevitch.net \
    --cc=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jit@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).