public inbox for jit@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Marc Nieper-Wißkirchen" <marc.nieper+gnu@gmail.com>
To: "Marc Nieper-Wißkirchen" <marc.nieper+gnu@gmail.com>
Cc: "David Malcolm" <dmalcolm@redhat.com>,
	"Marc Nieper-Wißkirchen via Jit" <jit@gcc.gnu.org>
Subject: Re: C++ API: Vector arguments / error detection
Date: Tue, 1 Feb 2022 15:41:05 +0100	[thread overview]
Message-ID: <CAEYrNrTt8GQCBXPWc8yFA_+Yqc4yXgx3M57Gh=pk=1gT7NRacg@mail.gmail.com> (raw)
In-Reply-To: <CAEYrNrR9zLExkKKyBBoEbwTjCnDPr6G2hdb6QkK6tWUM+LYv4w@mail.gmail.com>

Actually, we should use C++ templating mechanism here:

For example,

gccjit::struct_ gccjit::context::new_struct_type (const std::string
&name, std::vector<field> &fields, gccjit::location loc)

should become

template <typename T>
gccjit::struct_ gccjit::context::new_struct_type (const std::string
&name, T &&fields, gccjit::location loc)
gccjit::struct_ gccjit::context::new_struct_type (const std::string
&name, std::initializer_list <field>, gccjit::location loc)

The implementation would then be as follows:

template <typename T>
gccjit::struct_ gccjit::context::
new_struct_type (const std::string &name, T &&fields, gccjit::location loc)
{
  field *as_array_of_wrappers = const_cast <field *> (std::data
(std::forward <T> (fields)));
  gcc_jit_field **as_array_of_ptrs =
    reinterpret_cast <gcc_jit_field **> (as_array_of_wrappers);
  return struct_ (gcc_jit_context_new_struct_type (m_inner_ctxt,
                           loc.get_inner_location (),
                           name.c_str (),
                           std::size (std::forward <T> (fields)),
                           as_array_of_ptrs));
}

gccjit::struct_ gccjit::context::
new_struct_type (const std::string &name, std::initializer_list
<field> fields, gccjit::location loc)
{
  return new_struct_type <> (fields);
}

With it, all the following initializations are possible:

std::vector <field> fields {field1, field2};
new_struct_type ("name", fields);

std::array <field, 2> fields {field1, field2};
new_struct_type ("name", fields);

field fields[2] {field1, field2};
new_struct_type ("name", fields);

new_struct_type ("name", {field1, field2}); // initializer list

Note that the above implementation uses std::data and std::size, which
is only available from C++17 onward, for clarity, but both template
functions can be easily provided by libgccjit++.h when used with C++11
or C++14.

}




Am Mo., 31. Jan. 2022 um 16:39 Uhr schrieb Marc Nieper-Wißkirchen
<marc.nieper+gnu@gmail.com>:
>
> Probably more important than changing std::vector <...> &args to const std::vector <...> &args is to offer an extra set of methods that take a std::initializer_list instead of a vector.  These should be allocated on the stack and not on the heap, improving efficiency when the argument lists are known ahead.
>
>
> Am Mo., 31. Jan. 2022 um 16:32 Uhr schrieb Marc Nieper-Wißkirchen <marc.nieper+gnu@gmail.com>:
>>
>> Am Mo., 31. Jan. 2022 um 16:20 Uhr schrieb David Malcolm <dmalcolm@redhat.com>:
>>>
>>> On Mon, 2022-01-31 at 16:08 +0100, Marc Nieper-Wißkirchen wrote:
>>> > Am Mo., 31. Jan. 2022 um 15:49 Uhr schrieb David Malcolm <
>>> > dmalcolm@redhat.com>:
>>> >
>>> > > On Sun, 2022-01-30 at 16:35 +0100, Marc Nieper-Wißkirchen via Jit
>>> > > wrote:
>>> > > > This is about two unrelated issues regarding the C++ API:
>>> > > >
>>> > > > (1) At the moment, a lot of API functions like new_function take
>>> > > > non-
>>> > > > const
>>> > > > referenced to vectors, e.g. std::vector <param> &params;
>>> > > >
>>> > > > Can we change this into const references?  This way, one can write
>>> > > >
>>> > > > auto param1 = ctxt.new_param (...);
>>> > > > auto param2 = ctxt.new_param (...);
>>> > > > auto func = new_function (..., {param1, param2}, ...);
>>> > > >
>>> > > > instead of the more complicated and less convenient
>>> > > >
>>> > > > auto param1 = ...;
>>> > > > auto param2 = ...;
>>> > > > auto params = std::vector {param1, param2};
>>> > > > auto func = new_function (..., params, ...);
>>> > >
>>> > > I wonder why they're not const already - maybe I just forgot?
>>> > >
>>> > > Making the refs const sounds like a good idea to me.
>>> > >
>>> >
>>> > The reason why you didn't make them const may be that the C API that is
>>> > called takes non-const arrays of pointers.  The latter is probably a
>>> > good
>>> > idea because of the const model in C and the const-cast restrictions.
>>>
>>> Yeah, that was probably it.
>>>
>>> >
>>> > But when the C API doesn't change the arrays, all that is needed is a
>>> > const_cast <...> (...) in the C++ API implementation.  If you can
>>> > confirm
>>> > the former, I can do the latter.
>>>
>>> Sounds like a good idea to me.
>>>
>>> > >
>>>
>>> > > > (2) When using gccjit::context::compile_to_file, how can I check
>>> > > > whether
>>> > > > the compilation succeeded without errors or not? In libgccjit++.h
>>> > > > no
>>> > > > error
>>> > > > is checked and no exception thrown.
>>> > >
>>> > > Looks like I forgot to:
>>> > > (a) give a return value to gcc_jit_context_compile_to_file, and
>>> > > (b) add C++ bindings for gcc_jit_context_get_first_error and
>>> > > gcc_jit_context_get_last_error.
>>> > >
>>> > > Looks like fixing (a) would be an ABI break, bother (perhaps we could
>>> > > add a revised gcc_jit_context_compile_to_file symbol and do it with
>>> > > symbol versioning)
>>> > >
>>> >
>>> > A workaround would be to check that the context is without an error
>>> > before
>>> > the context to gcc_jit_context_compile_to_file and then to check
>>> > afterward
>>> > again, right?
>>>
>>> That could work.
>>>
>>> > If this is true, a return value (while nice) is not
>>> > absolutely necessary and an ABI break would not be needed.  (By the
>>> > way, is
>>> > changing the return type from void to int, say, an ABI break on any
>>> > platform?
>>>
>>> I'm not sure; I think I'm erring on the side of caution.
>>>
>>>
>>> >
>>> > The C++ API could throw an error whenever there is an error in the
>>> > context
>>> > after compiling.
>>>
>>> Sounds good.
>>
>>
>> Thanks for all these prompt responses.  You can expect some patches during the next few days. :)
>>
>>
>>>
>>>
>>> >
>>> >
>>> > > With those in place, it looks like either the client code needs to
>>> > > check gcc::jit::context::get_last_error on failure, or, better, it
>>> > > should probably change to look something like this (untested):
>>> > >
>>> > > inline void
>>> > > context::compile_to_file (enum gcc_jit_output_kind output_kind,
>>> > >                           const char *output_path)
>>> > > {
>>> > >   if (gcc_jit_context_compile_to_file (m_inner_ctxt,
>>> > >                                        output_kind,
>>> > >                                        output_path))
>>> > >     throw error (m_inner_ctxt);
>>> > > }
>>> > >
>>> > > where error's ctor should call gcc::jit::context::get_last_error on
>>> > > the
>>> > > ctxt, or something similar to this.
>>> > >
>>> > >
>>> > > Sorry about the C++ API being much less polished that the C API.
>>> > >
>>> >
>>> > There absolutely no need to excuse :).  libgccjit is a great idea and
>>> > piece
>>> > of software.  That's the nice thing about free software that people who
>>> > need a more polished API can contribute.
>>> > Marc
>>>
>>> Indeed.  I was tempted to say "patches welcome", but that can come
>>> across as rather passive-aggressive :)
>>
>>
>> :) "Welcome" is a nice and friendly word, isn't it?
>>
>>>
>>> Out of curiosity, are you using the C++ API for something, and would
>>> you like it on the "Who's using this code?" section on
>>> https://gcc.gnu.org/wiki/JIT ?
>>
>>
>> My "something" is far from finished.  But I will come back to you when I have something to show.  What I can say so far is that the C++ API really saves a lot of typing.
>>
>> Marc
>>

      reply	other threads:[~2022-02-01 14:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-30 15:35 Marc Nieper-Wißkirchen
2022-01-31 14:49 ` David Malcolm
2022-01-31 15:08   ` Marc Nieper-Wißkirchen
2022-01-31 15:19     ` David Malcolm
2022-01-31 15:32       ` Marc Nieper-Wißkirchen
2022-01-31 15:39         ` Marc Nieper-Wißkirchen
2022-02-01 14:41           ` Marc Nieper-Wißkirchen [this message]

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='CAEYrNrTt8GQCBXPWc8yFA_+Yqc4yXgx3M57Gh=pk=1gT7NRacg@mail.gmail.com' \
    --to=marc.nieper+gnu@gmail.com \
    --cc=dmalcolm@redhat.com \
    --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).