From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb36.google.com (mail-yb1-xb36.google.com [IPv6:2607:f8b0:4864:20::b36]) by sourceware.org (Postfix) with ESMTPS id 8A98E3858033 for ; Tue, 1 Feb 2022 14:41:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8A98E3858033 Received: by mail-yb1-xb36.google.com with SMTP id r65so51500231ybc.11 for ; Tue, 01 Feb 2022 06:41:18 -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:content-transfer-encoding; bh=STU4xThhKAozq/WtDT+YJ1TYW7FtQnjOLCDYRi2dquI=; b=BWVvPp+5tFpfHqgAkakcpMjdtr1dfFASMWFWab6TbepyoPGpgk7SqhjdT8wj4Cho/t vP7QBHwdkmEHExOWfHk6fGwXvC1knVaeBPhwilu+TrIlEDbR7lR7x6u7cch01RU49tI7 vT2cZSpisV5/6kEOMOnbzo/q7xi2fKVrBk9Ll4dDuV2GUikNdPh2EAchRPZ6XJEb0/PG NzedqkHPnIMJBxO702hAwWanUYUEnQ/4Jq8N6Ie2k+tDDze4uiN5ZEIJ0xMtpxQyQ2VP HV09zaqpI1dEpXDh+dVvChbL14Q4U0lALeO9zC6WhK6Kd+kwrx0aFDL3yHpqodjpGBjY 0ixw== X-Gm-Message-State: AOAM533KIG5tJvGuRwh9ygtluXUhAceExy7zbcfrxm0bCJj5BZgfXp8K noTkgCxFxSE2i45yRrE81h+3XW/IdY9JnD3asN4= X-Google-Smtp-Source: ABdhPJz84dRrNZcNyKaMfZdUBl/zD4IcplqsyTymTWzWJbFvF2YAyMWxSwh53z2X39dyuZ/h3XePX13BLQUHJDgxlDs= X-Received: by 2002:a25:9746:: with SMTP id h6mr25220660ybo.389.1643726477892; Tue, 01 Feb 2022 06:41:17 -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: Tue, 1 Feb 2022 15:41:05 +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?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=0.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: Tue, 01 Feb 2022 14:41:21 -0000 Actually, we should use C++ templating mechanism here: For example, gccjit::struct_ gccjit::context::new_struct_type (const std::string &name, std::vector &fields, gccjit::location loc) should become template 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 , gccjit::location loc) The implementation would then be as follows: template gccjit::struct_ gccjit::context:: new_struct_type (const std::string &name, T &&fields, gccjit::location loc) { field *as_array_of_wrappers =3D const_cast (std::data (std::forward (fields))); gcc_jit_field **as_array_of_ptrs =3D reinterpret_cast (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 (fields)), as_array_of_ptrs)); } gccjit::struct_ gccjit::context:: new_struct_type (const std::string &name, std::initializer_list fields, gccjit::location loc) { return new_struct_type <> (fields); } With it, all the following initializations are possible: std::vector fields {field1, field2}; new_struct_type ("name", fields); std::array 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=C3=9Fkirchen : > > Probably more important than changing std::vector <...> &args to const st= d::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 sta= ck and not on the heap, improving efficiency when the argument lists are kn= own ahead. > > > Am Mo., 31. Jan. 2022 um 16:32 Uhr schrieb Marc Nieper-Wi=C3=9Fkirchen : >> >> Am Mo., 31. Jan. 2022 um 16:20 Uhr schrieb David Malcolm : >>> >>> 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 = 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 ¶ms; >>> > > > >>> > > > Can we change this into const references? This way, one can writ= e >>> > > > >>> > > > 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 = 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 cou= ld >>> > > 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 duri= ng 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 w= ho >>> > 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 >>