From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb33.google.com (mail-yb1-xb33.google.com [IPv6:2607:f8b0:4864:20::b33]) by sourceware.org (Postfix) with ESMTPS id D0B7B3850412 for ; Mon, 31 Jan 2022 15:39:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D0B7B3850412 Received: by mail-yb1-xb33.google.com with SMTP id k31so41710891ybj.4 for ; Mon, 31 Jan 2022 07:39:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Bo/5kEaqG3bZVm/odKD6jRhBbeNPW6LTcHveNhnpCfk=; b=2/jdDPLbTxInO08eNSsrNJmNCZJgxO0DWBncM0BOTjiZ5lvKiJZKS5syjAJ9rwDBLl rflHAZIspq/W95p6D9U9zt9EPlxdKyMlNnz/hPcBEZ6RBexKU+gjDnG4ZdKb/0U1j30n dKU4Oh96lQ8val1NH43p+9R4ZC8h6VLfsH/G3tRORP1rjminlTdGBGlG1kykksWs7Ivd tb0bNfPumXKTzJXolAzglMee1Fse2VWlrhFNNTH2UxwrySWtthWC7BPPK5+Ysh4TlkAc BOMfeR2ZwUtU/nAq3mDioEtmXK2QH/lnVvPeno8MP0ZmsmhVoRi5bbbeNxPKwzhNOqBa w4Rw== X-Gm-Message-State: AOAM531ArbernJp7im+OXERo4oK/OXgy5SGa41OLI5qCNIeLKk3hF+/3 JAt/CfM/M8HiD60cTVFW0RFYpebQByv0gXRJ5OY= X-Google-Smtp-Source: ABdhPJzQyVY2q4GFG8+hLfvwnqA0frPlvBuFiD9yzPDo3KG12H9LgofIqvd9Ig0ROyDkJqA6jvjXnl4eSAHhSUsN4x4= X-Received: by 2002:a25:5c7:: with SMTP id 190mr29187619ybf.554.1643643586243; Mon, 31 Jan 2022 07:39:46 -0800 (PST) MIME-Version: 1.0 References: <1e83575c7d369fa96844b551f8606922fc8e0627.camel@redhat.com> <78ed3281f7beec5f37be521f04ce44a8d8690867.camel@redhat.com> In-Reply-To: From: =?UTF-8?Q?Marc_Nieper=2DWi=C3=9Fkirchen?= Date: Mon, 31 Jan 2022 16:39:35 +0100 Message-ID: Subject: Re: C++ API: Vector arguments / error detection To: =?UTF-8?Q?Marc_Nieper=2DWi=C3=9Fkirchen?= Cc: David Malcolm , =?UTF-8?Q?Marc_Nieper=2DWi=C3=9Fkirchen_via_Jit?= X-Spam-Status: No, score=0.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, FREEMAIL_REPLY, HTML_MESSAGE, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Jan 2022 15:39:49 -0000 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=C3=9Fkirchen < 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=C3=9Fkirchen 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=C3=9Fkirchen via J= it >> > > 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 ¶ms; >> > > > >> > > > Can we change this into const references? This way, one can write >> > > > >> > > > auto param1 =3D ctxt.new_param (...); >> > > > auto param2 =3D ctxt.new_param (...); >> > > > auto func =3D new_function (..., {param1, param2}, ...); >> > > > >> > > > instead of the more complicated and less convenient >> > > > >> > > > auto param1 =3D ...; >> > > > auto param2 =3D ...; >> > > > auto params =3D std::vector {param1, param2}; >> > > > auto func =3D 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 i= s >> > 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 coul= d >> > > 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 durin= g > 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 wh= o >> > 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 > >