public inbox for jit@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@redhat.com>
To: "Marc Nieper-Wißkirchen" <marc.nieper+gnu@gmail.com>
Cc: "Marc Nieper-Wißkirchen via Jit" <jit@gcc.gnu.org>
Subject: Re: C++ API: Vector arguments / error detection
Date: Mon, 31 Jan 2022 10:19:58 -0500	[thread overview]
Message-ID: <78ed3281f7beec5f37be521f04ce44a8d8690867.camel@redhat.com> (raw)
In-Reply-To: <CAEYrNrS3bK2atV=v=i5579a530uB_kvRWk4eepb6gWDCf8gAPA@mail.gmail.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.

> 
> 
> > 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 :)

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 ?


Thanks
Dave



  reply	other threads:[~2022-01-31 15:20 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 [this message]
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

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=78ed3281f7beec5f37be521f04ce44a8d8690867.camel@redhat.com \
    --to=dmalcolm@redhat.com \
    --cc=jit@gcc.gnu.org \
    --cc=marc.nieper+gnu@gmail.com \
    /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).