* Re: JIT patch: add gcc_jit_magic_int
2016-01-01 0:00 ` Andrew Pinski
@ 2016-01-01 0:00 ` Basile Starynkevitch
2016-01-01 0:00 ` Andrew Pinski
1 sibling, 0 replies; 6+ messages in thread
From: Basile Starynkevitch @ 2016-01-01 0:00 UTC (permalink / raw)
To: Andrew Pinski; +Cc: GCC Patches, jit, David Malcolm
On 06/07/2016 09:19 PM, Andrew Pinski wrote:
> On Mon, May 23, 2016 at 5:26 AM, Basile Starynkevitch
> <basile@starynkevitch.net> wrote:
>> Hello All,
>>
>> As I explained in https://gcc.gnu.org/ml/jit/2016-q2/msg00042.html it is
>> difficult (or tricky without using dirty tricks involving the GCC plugin
>> headers) to use GCCJIT to emit code equivalent to the following C file:
>>
>> extern int a;
>> int get_atomic_a (void) {
>> return __atomic_load_n (&a, __ATOMIC_SEQ_CST);
>> }
>>
>> The issue is that __ATOMIC_SEQ_CST is a magic preprocessor (but
>> non-standard!) symbol which might not be available
>> (or might have a different value) in the C code for GCCJIT building such an
>> AST.
>> So we need a function to retrieve some magic integral value from the GCCJIT
>> compiler.
> Huh? Why can't you just use the enum:
> enum memmodel
> {
> MEMMODEL_RELAXED = 0,
> MEMMODEL_CONSUME = 1,
> MEMMODEL_ACQUIRE = 2,
> MEMMODEL_RELEASE = 3,
> MEMMODEL_ACQ_REL = 4,
> MEMMODEL_SEQ_CST = 5,
> MEMMODEL_LAST = 6,
> MEMMODEL_SYNC_ACQUIRE = MEMMODEL_ACQUIRE | MEMMODEL_SYNC,
> MEMMODEL_SYNC_RELEASE = MEMMODEL_RELEASE | MEMMODEL_SYNC,
> MEMMODEL_SYNC_SEQ_CST = MEMMODEL_SEQ_CST | MEMMODEL_SYNC
> };
>
(sorry for the late reply)
I think that Andrew Pinski is right, and that we should make that enum
available in libgccjit.h.
David Malcolm, please tell us how should that be done? Would you accept
a #include "coretypes.h" inside libgccjit.h? Or should I copy paste the
enum memmodel into libgccjit.h?
Cheers.
--
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} ***
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: JIT patch: add gcc_jit_magic_int
2016-01-01 0:00 ` Andrew Pinski
@ 2016-01-01 0:00 ` Andrew Pinski
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Pinski @ 2016-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: GCC Patches, jit, David Malcolm
On Tue, Jun 7, 2016 at 12:22 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Jun 7, 2016 at 12:19 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Mon, May 23, 2016 at 5:26 AM, Basile Starynkevitch
>> <basile@starynkevitch.net> wrote:
>>> Hello All,
>>>
>>> As I explained in https://gcc.gnu.org/ml/jit/2016-q2/msg00042.html it is
>>> difficult (or tricky without using dirty tricks involving the GCC plugin
>>> headers) to use GCCJIT to emit code equivalent to the following C file:
>>>
>>> extern int a;
>>> int get_atomic_a (void) {
>>> return __atomic_load_n (&a, __ATOMIC_SEQ_CST);
>>> }
>>>
>>> The issue is that __ATOMIC_SEQ_CST is a magic preprocessor (but
>>> non-standard!) symbol which might not be available
>>> (or might have a different value) in the C code for GCCJIT building such an
>>> AST.
>>
>>>
>>> So we need a function to retrieve some magic integral value from the GCCJIT
>>> compiler.
>>
>> Huh? Why can't you just use the enum:
>> enum memmodel
>> {
>> MEMMODEL_RELAXED = 0,
>> MEMMODEL_CONSUME = 1,
>> MEMMODEL_ACQUIRE = 2,
>> MEMMODEL_RELEASE = 3,
>> MEMMODEL_ACQ_REL = 4,
>> MEMMODEL_SEQ_CST = 5,
>> MEMMODEL_LAST = 6,
>> MEMMODEL_SYNC_ACQUIRE = MEMMODEL_ACQUIRE | MEMMODEL_SYNC,
>> MEMMODEL_SYNC_RELEASE = MEMMODEL_RELEASE | MEMMODEL_SYNC,
>> MEMMODEL_SYNC_SEQ_CST = MEMMODEL_SEQ_CST | MEMMODEL_SYNC
>> };
>>
>>
>> For the values? I don't understand why you need to have a direct
>> relationship to preprocessed macros to these values here.
>> Have your own lookup table inside your own JIT rather than a generic
>> way of doing it.
>> Having a generic way seems more incorrect way of doing it and even
>> says that the JIT is supposed to JITting C code but it is not JITing C
>> code.
>
>
> That is move enum memmodel to be included in the JIT API too.
Or rather have an API which the __atomic_* for you and all you need to
supply is the the "JIT" memory model and the locations.
Thanks,
Andrew
>
>>
>> Thanks,
>> Andrew Pinski
>>
>>>
>>> The attached patch (relative to trunk svn 236583) is a first attempt to
>>> solve that issue
>>> (and also give ability to query some other magic numbers).
>>>
>>> Proposed ChangeLog entry (in gcc/jit/)
>>>
>>> 2016-05-23 Basile Starynkevitch <basile@starynkevitch.net>
>>> * libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_magic_int): New macro.
>>> (gcc_jit_magic_int): New public function declaration.
>>>
>>> * libgccjit.c: Include "cppbuiltin.h", "options.h", "flag-types.h"
>>> (gcc_jit_magic_int): New function.
>>>
>>> * libgccjit.map: Add gcc_jit_magic_int to LIBGCCJIT_ABI_6.
>>>
>>> Comments (or an ok to commit) are welcome. (I am not sure that
>>> __SANITIZE_ADDRESS__ is correctly handled,
>>> because I would believe that optimization flags are not globals in GCCJIT)
>>>
>>> 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} ***
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* JIT patch: add gcc_jit_magic_int
@ 2016-01-01 0:00 Basile Starynkevitch
2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` David Malcolm
0 siblings, 2 replies; 6+ messages in thread
From: Basile Starynkevitch @ 2016-01-01 0:00 UTC (permalink / raw)
To: gcc-patches, jit, David Malcolm
[-- Attachment #1: Type: text/plain, Size: 1598 bytes --]
Hello All,
As I explained in https://gcc.gnu.org/ml/jit/2016-q2/msg00042.html it is
difficult (or tricky without using dirty tricks involving the GCC plugin
headers) to use GCCJIT to emit code equivalent to the following C file:
extern int a;
int get_atomic_a (void) {
return __atomic_load_n (&a, __ATOMIC_SEQ_CST);
}
The issue is that __ATOMIC_SEQ_CST is a magic preprocessor (but non-standard!) symbol which might not be available
(or might have a different value) in the C code for GCCJIT building such an AST.
So we need a function to retrieve some magic integral value from the GCCJIT compiler.
The attached patch (relative to trunk svn 236583) is a first attempt to solve that issue
(and also give ability to query some other magic numbers).
Proposed ChangeLog entry (in gcc/jit/)
2016-05-23 Basile Starynkevitch <basile@starynkevitch.net>
* libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_magic_int): New macro.
(gcc_jit_magic_int): New public function declaration.
* libgccjit.c: Include "cppbuiltin.h", "options.h", "flag-types.h"
(gcc_jit_magic_int): New function.
* libgccjit.map: Add gcc_jit_magic_int to LIBGCCJIT_ABI_6.
Comments (or an ok to commit) are welcome. (I am not sure that __SANITIZE_ADDRESS__ is correctly handled,
because I would believe that optimization flags are not globals in GCCJIT)
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: trunk236583-jitmagicint.diff --]
[-- Type: text/x-patch, Size: 3493 bytes --]
Index: gcc/jit/libgccjit.h
===================================================================
--- gcc/jit/libgccjit.h (revision 236583)
+++ gcc/jit/libgccjit.h (working copy)
@@ -1387,6 +1387,27 @@
gcc_jit_rvalue_set_bool_require_tail_call (gcc_jit_rvalue *call,
int require_tail_call);
+
+ /* Magical integer values useful in the compiler; similar to
+ predefined C macros like __GNUC__, __GNUC_MINOR__,
+ __GNUC_PATCHLEVEL__, __ATOMIC_RELAXED, __ATOMIC_SEQ_CST,
+ __ATOMIC_ACQUIRE, __ATOMIC_RELEASE, __ATOMIC_ACQ_REL,
+ __ATOMIC_CONSUME, __PIC__, __PIE__, etc. Typical usage would be:
+
+ bool err=false;
+ int mypic = gcc_jit_magic_int("__PIC__", &err);
+ if (err) somethinggotwrong();
+
+ This function is expected to be rarely called, typically once at
+ initialization time.
+
+ This API entrypoint was added in LIBGCCJIT_ABI_6; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_magic_int
+ */
+#define LIBGCCJIT_HAVE_gcc_jit_magic_int
+extern int gcc_jit_magic_int(const char*name, bool*errp);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: gcc/jit/libgccjit.c
===================================================================
--- gcc/jit/libgccjit.c (revision 236583)
+++ gcc/jit/libgccjit.c (working copy)
@@ -23,6 +23,9 @@
#include "coretypes.h"
#include "timevar.h"
#include "typed-splay-tree.h"
+#include "cppbuiltin.h"
+#include "options.h"
+#include "flag-types.h"
#include "libgccjit.h"
#include "jit-recording.h"
@@ -2970,3 +2973,44 @@
call->set_require_tail_call (require_tail_call);
}
+
+
+/* Public entrypoint. See description in libgccjit.h. */
+
+int gcc_jit_magic_int(const char*name, bool*errp)
+{
+ static int major, minor, patchlevel;
+ if (!major) /* call once: */
+ parse_basever (&major, &minor, &patchlevel);
+
+ RETURN_VAL_IF_FAIL (name,
+ errp?((*errp=true),0):0,
+ NULL, NULL,
+ "NULL name");
+ RETURN_VAL_IF_FAIL (name[0] == '_' && name[1] == '_',
+ errp?((*errp=true),0):0,
+ NULL, NULL,
+ "name should start with two underscores");
+#define HAVE_MAGIC_INT(NamStr,Val) do { \
+ if (!strcmp(name, NamStr)) { \
+ if (errp) *errp = false; \
+ return Val; }} while(0)
+ // keep these in alphabetical order...
+ HAVE_MAGIC_INT("__ATOMIC_ACQUIRE", MEMMODEL_ACQUIRE);
+ HAVE_MAGIC_INT("__ATOMIC_ACQ_REL", MEMMODEL_ACQ_REL);
+ HAVE_MAGIC_INT("__ATOMIC_CONSUME", MEMMODEL_CONSUME);
+ HAVE_MAGIC_INT("__ATOMIC_RELAXED", MEMMODEL_RELAXED);
+ HAVE_MAGIC_INT("__ATOMIC_RELEASE", MEMMODEL_RELEASE);
+ HAVE_MAGIC_INT("__ATOMIC_SEQ_CST", MEMMODEL_SEQ_CST);
+ HAVE_MAGIC_INT("__GNUC_MINOR__", minor);
+ HAVE_MAGIC_INT("__GNUC_PATCHLEVEL__", patchlevel);
+ HAVE_MAGIC_INT("__GNUC__", major);
+ HAVE_MAGIC_INT("__PIC__", flag_pic);
+ HAVE_MAGIC_INT("__PIE__", flag_pie);
+ HAVE_MAGIC_INT("__SANITIZE_ADDRESS__", flag_sanitize & SANITIZE_ADDRESS);
+ HAVE_MAGIC_INT("__SANITIZE_THREAD__", flag_sanitize & SANITIZE_THREAD);
+#undef HAVE_MAGIC_INT
+ RETURN_VAL_IF_FAIL_PRINTF1 (false, errp?((*errp=true),0):0,
+ NULL, NULL,
+ "unknown magic int name: %s", name);
+}
Index: gcc/jit/libgccjit.map
===================================================================
--- gcc/jit/libgccjit.map (revision 236583)
+++ gcc/jit/libgccjit.map (working copy)
@@ -149,4 +149,5 @@
LIBGCCJIT_ABI_6 {
global:
gcc_jit_rvalue_set_bool_require_tail_call;
+ gcc_jit_magic_int;
} LIBGCCJIT_ABI_5;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: JIT patch: add gcc_jit_magic_int
2016-01-01 0:00 JIT patch: add gcc_jit_magic_int Basile Starynkevitch
2016-01-01 0:00 ` Andrew Pinski
@ 2016-01-01 0:00 ` David Malcolm
1 sibling, 0 replies; 6+ messages in thread
From: David Malcolm @ 2016-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch, gcc-patches, jit
On Mon, 2016-05-23 at 14:26 +0200, Basile Starynkevitch wrote:
> Hello All,
>
> As I explained in https://gcc.gnu.org/ml/jit/2016-q2/msg00042.html it
> is
> difficult (or tricky without using dirty tricks involving the GCC
> plugin
> headers) to use GCCJIT to emit code equivalent to the following C
> file:
>
> extern int a;
> int get_atomic_a (void) {
> return __atomic_load_n (&a, __ATOMIC_SEQ_CST);
> }
>
> The issue is that __ATOMIC_SEQ_CST is a magic preprocessor (but non
> -standard!) symbol which might not be available
> (or might have a different value) in the C code for GCCJIT building
> such an AST.
>
> So we need a function to retrieve some magic integral value from the
> GCCJIT compiler.
>
> The attached patch (relative to trunk svn 236583) is a first attempt
> to solve that issue
> (and also give ability to query some other magic numbers).
>
> Proposed ChangeLog entry (in gcc/jit/)
>
> 2016-05-23 Basile Starynkevitch <basile@starynkevitch.net>
> * libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_magic_int): New macro.
> (gcc_jit_magic_int): New public function declaration.
>
> * libgccjit.c: Include "cppbuiltin.h", "options.h", "flag
> -types.h"
> (gcc_jit_magic_int): New function.
>
> * libgccjit.map: Add gcc_jit_magic_int to LIBGCCJIT_ABI_6.
>
> Comments (or an ok to commit) are welcome. (I am not sure that
> __SANITIZE_ADDRESS__ is correctly handled,
> because I would believe that optimization flags are not globals in
> GCCJIT)
Sorry about the delay in responding to this.
> Index: gcc/jit/libgccjit.h
> ===================================================================
> --- gcc/jit/libgccjit.h (revision 236583)
> +++ gcc/jit/libgccjit.h (working copy)
> @@ -1387,6 +1387,27 @@
> gcc_jit_rvalue_set_bool_require_tail_call (gcc_jit_rvalue *call,
> int require_tail_call);
>
> +
> + /* Magical integer values useful in the compiler; similar to
> + predefined C macros like __GNUC__, __GNUC_MINOR__,
> + __GNUC_PATCHLEVEL__, __ATOMIC_RELAXED, __ATOMIC_SEQ_CST,
> + __ATOMIC_ACQUIRE, __ATOMIC_RELEASE, __ATOMIC_ACQ_REL,
> + __ATOMIC_CONSUME, __PIC__, __PIE__, etc. Typical usage would
be:
> +
> + bool err=false;
> + int mypic = gcc_jit_magic_int("__PIC__", &err);
> + if (err) somethinggotwrong();
> +
> + This function is expected to be rarely called, typically once at
> + initialization time.
> +
> + This API entrypoint was added in LIBGCCJIT_ABI_6; you can test
for its
> + presence using
> + #ifdef LIBGCCJIT_HAVE_gcc_jit_magic_int
> + */
> +#define LIBGCCJIT_HAVE_gcc_jit_magic_int
> +extern int gcc_jit_magic_int(const char*name, bool*errp);
Does the client code need to be able to check to see if a name is
defined, or can they assume it always is?
If the latter, how about:
extern int
gcc_jit_context_get_predefined_int (gcc_jit_context *ctxt,
const char *name);
and have it set an error on the context if the name isn't
recognized?
If they need to be able to check it, I prefer:
extern int
gcc_jit_context_get_predefined_int (gcc_jit_context *ctxt,
const char *name,
int *out);
i.e. have the return value contain the "was name recognized" flag
and *out be written to (I'd use "bool", but we don't require its
availability).
I'm not fond of "magic int", but I'm not fond of my proposed name
either.
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
> Index: gcc/jit/libgccjit.c
> ===================================================================
> --- gcc/jit/libgccjit.c (revision 236583)
> +++ gcc/jit/libgccjit.c (working copy)
> @@ -23,6 +23,9 @@
> #include "coretypes.h"
> #include "timevar.h"
> #include "typed-splay-tree.h"
> +#include "cppbuiltin.h"
> +#include "options.h"
> +#include "flag-types.h"
>
> #include "libgccjit.h"
> #include "jit-recording.h"
> @@ -2970,3 +2973,44 @@
>
> call->set_require_tail_call (require_tail_call);
> }
> +
> +
> +/* Public entrypoint. See description in libgccjit.h. */
> +
> +int gcc_jit_magic_int(const char*name, bool*errp)
> +{
> + static int major, minor, patchlevel;
> + if (!major) /* call once: */
> + parse_basever (&major, &minor, &patchlevel);
These vars are global state, and they've not guarded by a mutex.
Can they be moved to the gcc_jit_context instead? (actually,
to gcc::jit::recording::context). That way they'd be covered by
the general policies on thread-safety here:
https://gcc.gnu.org/onlinedocs/jit/topics/contexts.html#thread-safety
That said, see the question below about the "__GNUC_*" vars.
> +
> + RETURN_VAL_IF_FAIL (name,
> + errp?((*errp=true),0):0,
The above line is too terse for my taste.
> + NULL, NULL,
> + "NULL name");
> + RETURN_VAL_IF_FAIL (name[0] == '_' && name[1] == '_',
> + errp?((*errp=true),0):0,
> + NULL, NULL,
> + "name should start with two underscores");
> +#define HAVE_MAGIC_INT(NamStr,Val) do { \
Please use block caps for the macro arguments.
> + if (!strcmp(name, NamStr)) { \
> + if (errp) *errp = false; \
> + return Val; }} while(0)
> + // keep these in alphabetical order...
Please use a C-style comment here.
> + HAVE_MAGIC_INT("__ATOMIC_ACQUIRE", MEMMODEL_ACQUIRE);
> + HAVE_MAGIC_INT("__ATOMIC_ACQ_REL", MEMMODEL_ACQ_REL);
> + HAVE_MAGIC_INT("__ATOMIC_CONSUME", MEMMODEL_CONSUME);
> + HAVE_MAGIC_INT("__ATOMIC_RELAXED", MEMMODEL_RELAXED);
> + HAVE_MAGIC_INT("__ATOMIC_RELEASE", MEMMODEL_RELEASE);
> + HAVE_MAGIC_INT("__ATOMIC_SEQ_CST", MEMMODEL_SEQ_CST);
It sounds like we have a concrete use-case for these.
> + HAVE_MAGIC_INT("__GNUC_MINOR__", minor);
> + HAVE_MAGIC_INT("__GNUC_PATCHLEVEL__", patchlevel);
> + HAVE_MAGIC_INT("__GNUC__", major);
What's the use-case for these? It seems a bit weird to expose
them as "GNU C" when libgccjit isn't C.
> + HAVE_MAGIC_INT("__PIC__", flag_pic);
> + HAVE_MAGIC_INT("__PIE__", flag_pie);
> + HAVE_MAGIC_INT("__SANITIZE_ADDRESS__", flag_sanitize &
SANITIZE_ADDRESS);
> + HAVE_MAGIC_INT("__SANITIZE_THREAD__", flag_sanitize &
SANITIZE_THREAD);
These variables are determined by command-line options, and hence
are going to vary based on gcc_jit_context_add_command_line_option
in a way that's difficult to implement from libgccjit.c; the command
line options effectively get values in jit-playback.c when we
construct an argv for toplev.
> +#undef HAVE_MAGIC_INT
> + RETURN_VAL_IF_FAIL_PRINTF1 (false, errp?((*errp=true),0):0,
> + NULL, NULL,
> + "unknown magic int name: %s", name);
> +}
> Index: gcc/jit/libgccjit.map
> ===================================================================
> --- gcc/jit/libgccjit.map (revision 236583)
> +++ gcc/jit/libgccjit.map (working copy)
> @@ -149,4 +149,5 @@
> LIBGCCJIT_ABI_6 {
> global:
> gcc_jit_rvalue_set_bool_require_tail_call;
> + gcc_jit_magic_int;
> } LIBGCCJIT_ABI_5;
>
Please create a fresh ABI tag for the new symbol.
The patch is missing test cases; it needs to have at least 2
(one showing success, one showing failure).
A full patch would also need documentation.
It strikes me that if we add new names to this in the future,
we'd have no way to introspect client binaries to see which
version of the entrypoint they're expecting. I don't know
if this is a problem. One very different approach that would
solve this would be to have one symbol per magic int, maybe
for them to simply be const data, rather than functions:
#define LIBGCCJIT_HAVE_gcc_jit__ATOMIC_ACQUIRE
extern const int gcc_jit__ATOMIC_ACQUIRE;
#define LIBGCCJIT_HAVE_gcc_jit__ATOMIC_ACQ_REL
extern const int gcc_jit__ATOMIC_ACQ_REL;
/* etc */
and to add these to libgccjit.map.
I'm not sure if the above is a good idea. It seems much simpler that
having an API entrypoint, but maybe there are drawbacks I've not
thought of.
Thoughts?
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: JIT patch: add gcc_jit_magic_int
2016-01-01 0:00 JIT patch: add gcc_jit_magic_int Basile Starynkevitch
@ 2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` Basile Starynkevitch
2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` David Malcolm
1 sibling, 2 replies; 6+ messages in thread
From: Andrew Pinski @ 2016-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: GCC Patches, jit, David Malcolm
On Mon, May 23, 2016 at 5:26 AM, Basile Starynkevitch
<basile@starynkevitch.net> wrote:
> Hello All,
>
> As I explained in https://gcc.gnu.org/ml/jit/2016-q2/msg00042.html it is
> difficult (or tricky without using dirty tricks involving the GCC plugin
> headers) to use GCCJIT to emit code equivalent to the following C file:
>
> extern int a;
> int get_atomic_a (void) {
> return __atomic_load_n (&a, __ATOMIC_SEQ_CST);
> }
>
> The issue is that __ATOMIC_SEQ_CST is a magic preprocessor (but
> non-standard!) symbol which might not be available
> (or might have a different value) in the C code for GCCJIT building such an
> AST.
>
> So we need a function to retrieve some magic integral value from the GCCJIT
> compiler.
Huh? Why can't you just use the enum:
enum memmodel
{
MEMMODEL_RELAXED = 0,
MEMMODEL_CONSUME = 1,
MEMMODEL_ACQUIRE = 2,
MEMMODEL_RELEASE = 3,
MEMMODEL_ACQ_REL = 4,
MEMMODEL_SEQ_CST = 5,
MEMMODEL_LAST = 6,
MEMMODEL_SYNC_ACQUIRE = MEMMODEL_ACQUIRE | MEMMODEL_SYNC,
MEMMODEL_SYNC_RELEASE = MEMMODEL_RELEASE | MEMMODEL_SYNC,
MEMMODEL_SYNC_SEQ_CST = MEMMODEL_SEQ_CST | MEMMODEL_SYNC
};
For the values? I don't understand why you need to have a direct
relationship to preprocessed macros to these values here.
Have your own lookup table inside your own JIT rather than a generic
way of doing it.
Having a generic way seems more incorrect way of doing it and even
says that the JIT is supposed to JITting C code but it is not JITing C
code.
Thanks,
Andrew Pinski
>
> The attached patch (relative to trunk svn 236583) is a first attempt to
> solve that issue
> (and also give ability to query some other magic numbers).
>
> Proposed ChangeLog entry (in gcc/jit/)
>
> 2016-05-23 Basile Starynkevitch <basile@starynkevitch.net>
> * libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_magic_int): New macro.
> (gcc_jit_magic_int): New public function declaration.
>
> * libgccjit.c: Include "cppbuiltin.h", "options.h", "flag-types.h"
> (gcc_jit_magic_int): New function.
>
> * libgccjit.map: Add gcc_jit_magic_int to LIBGCCJIT_ABI_6.
>
> Comments (or an ok to commit) are welcome. (I am not sure that
> __SANITIZE_ADDRESS__ is correctly handled,
> because I would believe that optimization flags are not globals in GCCJIT)
>
> 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} ***
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: JIT patch: add gcc_jit_magic_int
2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` Basile Starynkevitch
@ 2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` Andrew Pinski
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Pinski @ 2016-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: GCC Patches, jit, David Malcolm
On Tue, Jun 7, 2016 at 12:19 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, May 23, 2016 at 5:26 AM, Basile Starynkevitch
> <basile@starynkevitch.net> wrote:
>> Hello All,
>>
>> As I explained in https://gcc.gnu.org/ml/jit/2016-q2/msg00042.html it is
>> difficult (or tricky without using dirty tricks involving the GCC plugin
>> headers) to use GCCJIT to emit code equivalent to the following C file:
>>
>> extern int a;
>> int get_atomic_a (void) {
>> return __atomic_load_n (&a, __ATOMIC_SEQ_CST);
>> }
>>
>> The issue is that __ATOMIC_SEQ_CST is a magic preprocessor (but
>> non-standard!) symbol which might not be available
>> (or might have a different value) in the C code for GCCJIT building such an
>> AST.
>
>>
>> So we need a function to retrieve some magic integral value from the GCCJIT
>> compiler.
>
> Huh? Why can't you just use the enum:
> enum memmodel
> {
> MEMMODEL_RELAXED = 0,
> MEMMODEL_CONSUME = 1,
> MEMMODEL_ACQUIRE = 2,
> MEMMODEL_RELEASE = 3,
> MEMMODEL_ACQ_REL = 4,
> MEMMODEL_SEQ_CST = 5,
> MEMMODEL_LAST = 6,
> MEMMODEL_SYNC_ACQUIRE = MEMMODEL_ACQUIRE | MEMMODEL_SYNC,
> MEMMODEL_SYNC_RELEASE = MEMMODEL_RELEASE | MEMMODEL_SYNC,
> MEMMODEL_SYNC_SEQ_CST = MEMMODEL_SEQ_CST | MEMMODEL_SYNC
> };
>
>
> For the values? I don't understand why you need to have a direct
> relationship to preprocessed macros to these values here.
> Have your own lookup table inside your own JIT rather than a generic
> way of doing it.
> Having a generic way seems more incorrect way of doing it and even
> says that the JIT is supposed to JITting C code but it is not JITing C
> code.
That is move enum memmodel to be included in the JIT API too.
>
> Thanks,
> Andrew Pinski
>
>>
>> The attached patch (relative to trunk svn 236583) is a first attempt to
>> solve that issue
>> (and also give ability to query some other magic numbers).
>>
>> Proposed ChangeLog entry (in gcc/jit/)
>>
>> 2016-05-23 Basile Starynkevitch <basile@starynkevitch.net>
>> * libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_magic_int): New macro.
>> (gcc_jit_magic_int): New public function declaration.
>>
>> * libgccjit.c: Include "cppbuiltin.h", "options.h", "flag-types.h"
>> (gcc_jit_magic_int): New function.
>>
>> * libgccjit.map: Add gcc_jit_magic_int to LIBGCCJIT_ABI_6.
>>
>> Comments (or an ok to commit) are welcome. (I am not sure that
>> __SANITIZE_ADDRESS__ is correctly handled,
>> because I would believe that optimization flags are not globals in GCCJIT)
>>
>> 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} ***
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-10 12:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-01 0:00 JIT patch: add gcc_jit_magic_int Basile Starynkevitch
2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` Basile Starynkevitch
2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` Andrew Pinski
2016-01-01 0:00 ` David Malcolm
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).