Add support for setting the inline attribute

[antoyo]

@Commeownist Here's the work I started for rust-lang/rustc_codegen_gcc#87 . There are some issues noted as FIXME in the code, though.

I can't remember if we have access to c-family in libgccjit, so I don't know if we could have a gcc_jit_function_add_attribute function as you suggested.

I still think it's worth having functions specific to inlining because I believe the inline keyword needs to be set separately for #[inline].

[bjorn3]

There is a recursive #[inline(always)] function in libcore. Maybe that is the problem? In rust even #[inline(always)] is just a hint that should be ignored if it is impossible to inline.

[ghost]

As a quick workaround, we could check in rustc_cg_gcc if the function is recursive and then refuse to emit any inline attributes for it. I highly doubt that DECL_DISREGARD_INLINE_LIMITS is a good idea here. I'll test tomorrow.

[antoyo]

@Commeownist Why not? The doc says This is equivalent to using the always_inline attribute without the required diagnostics if the function cannot be inlined., but I'm not sure what it actually means. I'll have to read more code.

[ghost]

@antoyo DECL_DISREGARD_INLINE_LIMITS is totally correct here, see this function. My mistake, confused it with something else.

[antoyo]

This function seems to actually check whether the function being inlined is recursive.

[ghost]

I've been secretly working on it yesterday. It's chaotic ATM so I'll publish it tomorrow, but here are my thoughts:

I can't remember if we have access to c-family in libgccjit, so I don't know if we could have a gcc_jit_function_add_attribute function as you suggested.

You don't need this access. If you take a hard look at playback::context::new_function, you'll see that attributes aren't really tied to C that much:

https://github.com/antoyo/gcc/blob/919b622dec9fb8a68ca42ea69bd91521d86da52b/gcc/jit/jit-playback.c#L548-L557

if you look at your own patch, you'll see that you also adding the always_inline attribute this way, so it's clearly an evidence that attributes can be set on demand ;)

So this is how I'd do it:

// C API
void gcc_jit_function_add_attribute (gcc_jit_function *func, const char *attr)
{
  func->add_attribute (attr);
}

// jit-recording.h

class function : public memento
{
public:
  // ... snip ...
  virtual void add_attribute (const char *attr)  // <= add attribute
  { m_attributes.safe_push(attr); }

private:
  location *m_loc;
  enum gcc_jit_function_kind m_kind;
  type *m_return_type;
  auto_vec<string> m_attributes; // <= stored list of attrs
  string *m_name;
  auto_vec<param *> m_params;
  int m_is_variadic;
  enum built_in_function m_builtin_id;
  auto_vec<local *> m_locals;
  auto_vec<block *> m_blocks;
  type *m_fn_ptr_type;
};

// jit-playback.c

playback::function* playback::context::new_function (
              location *loc,
	      enum gcc_jit_function_kind kind,
	      type *return_type,
              auto_vec<string> *attributes, // <= pass stored attrs
	      const char *name,
	      const auto_vec<param *> *params,
	      int is_variadic,
	      enum built_in_function builtin_id)

{
  // ... snip ...

  // apply attributes
  FOR_EACH_VEC_ELT (*attributes, i, attr) {
    DECL_ATTRIBUTES (fndecl) = tree_cons (
           get_identifier (attr), 
 	   NULL, 
 	   DECL_ATTRIBUTES (fndecl)); 
}

 

This was a quick pseudocode, but I think the idea is clear.

I think the attributes-driven approach is more powerful, especially if we'll want to extend this to other attributes in future. Some examples I predict will come up: the cold and hot attributes, maybe noipa, maybe nonnull, noreturn, maybe nothrow, used, and so on.

I still think it's worth having functions specific to inlining because I believe the inline keyword needs to be set separately for #[inline].

Yes, that's right. Keep in mind that always_inline attribute will mark function as inlineable whether it was marked as inline or not, and noinline will conflict with the inline keyword, see https://godbolt.org/z/W76jx5b4d

How about a hybrid approach? You add void set_inline_flag(function *func, bool flag) that's only responsible for the inline keyword, and always/never_inline are set via function attributes. This will resolve the inline topic and it will also open the road for other unresolved topics such as #[used].

[ghost]

How about a hybrid approach? You add void set_inline_flag(function *func, bool flag) that's only responsible for the inline keyword, and always/never_inline are set via function attributes. This will resolve the inline topic and it will also open the road for other unresolved topics such as #[used].

Alternatively, we could take your current approach - which I like because it prevents the conflicting situation of inline void __attribute((noinline)) foo(); - but still add the add_attribute function.

[antoyo]

Alternatively, we could take your current approach - which I like because it prevents the conflicting situation of inline void __attribute((noinline)) foo(); - but still add the add_attribute function.

Yup, I like this.
We might have to add some checks to avoid this scenario of inline noinline.