Andrew Pinski (pinskia) <[email protected]>) commented on the code:
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
Does it make sense to do this way or should it be a target specific hook to add
the ability to have new fn attributes here?
>From a GCC point of view I rather have hooks rather than target specific items
>in generic code.
I know rustc and clang handle the target specific code in general code all the
time but I think that is bad design.
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
Could you please explain how that API would look like?
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
There will be a range defined for in gcc_jit_fn_attribute for target specific
enums.
And then you have headers for the target you are compiling which defines
attributes in that range. And you dispatch to the hook for that for the target.
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
You mean something like:
```
enum gcc_jit_fn_attribute
{
GCC_JIT_FN_ATTRIBUTE_ALIAS,
GCC_JIT_FN_ATTRIBUTE_ALWAYS_INLINE,
GCC_JIT_FN_ATTRIBUTE_INLINE,
GCC_JIT_FN_ATTRIBUTE_NOINLINE,
GCC_JIT_FN_ATTRIBUTE_TARGET,
GCC_JIT_FN_ATTRIBUTE_USED,
GCC_JIT_FN_ATTRIBUTE_VISIBILITY,
GCC_JIT_FN_ATTRIBUTE_COLD,
GCC_JIT_FN_ATTRIBUTE_RETURNS_TWICE,
GCC_JIT_FN_ATTRIBUTE_PURE,
GCC_JIT_FN_ATTRIBUTE_CONST,
GCC_JIT_FN_ATTRIBUTE_WEAK,
GCC_JIT_FN_ATTRIBUTE_NONNULL,
#include "target-fn-attributes.def"
}
```
?
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
More like:
```
enum gcc_jit_fn_attribute
{
...
GCC_JIT_FN_ATTRIBUTE_TARGET_START,
};
#include "target-libgccjit.h"
```
And then inside target-libgccjit.h for the specific target, define a new enum
that starts at GCC_JIT_FN_ATTRIBUTE_TARGET_START.
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
`libgccjit` aims to provide a [strong ABI
guarantee](https://gcc.gnu.org/onlinedocs/jit/topics/compatibility.html). How
would you keep that guarantee with this approach?
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
@antoyo wrote in
https://forge.sourceware.org/gcc/gcc-TEST/pulls/109#issuecomment-3110:
> `libgccjit` aims to provide a [strong ABI
> guarantee](https://gcc.gnu.org/onlinedocs/jit/topics/compatibility.html). How
> would you keep that guarantee with this approach?
I don't see how that is NOT backwards compatible. Put
GCC_JIT_FN_ATTRIBUTE_TARGET_START higher up if needed so you can add more
inbetween. Like say `0x10_00_00`.
The ABI/API guarantee between targets is NOT talked about.
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
How would you handle the bindings from other programming languages to the C API
of libgccjit if we follow this approach?
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
How does they work right now? Is there some processing of the header file to
generate it?
If so that is how. Instead of processing the header file you process the
preprocessed source.
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
If @dmalcolm confirms that he's OK with having the libgccjit API different
depending on the target, I'll implement these changes.
I wonder if this could leads to problems or ABI breakages if at some point we
have a multi-arch GCC.
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
I think I prefer @antoyo's approach here. I'm not sure why we went with an
enum for identifying attributes in gcc_jit_function_add_attribute and
gcc_jit_function_add_string_attribute; perhaps using const char * would have
been better.
But given that we did, I prefer having a unified enum giving all possible
attributes - this might be better if we need to support cross-compilation in
the future.
> +++ gcc/jit/jit-playback.cc
> @@ -547,6 +547,38 @@ const char* fn_attribute_to_string (gcc_jit_fn_attribute
> attr)
> return "weak";
> case GCC_JIT_FN_ATTRIBUTE_NONNULL:
> return "nonnull";
> + case GCC_JIT_FN_ATTRIBUTE_ARM_CMSE_NONSECURE_CALL:
Patch looks good to me; thanks.
--
https://forge.sourceware.org/gcc/gcc-TEST/pulls/109#issuecomment-3100
