On Fri, Mar 21, 2025 at 6:43 PM Antoni Boucher <[email protected]> wrote: > > Hi David. > My gccrs patch was merged, so I was wondering if you could please review > this patch again. > > It was also posted here on GitHub: > https://github.com/antoyo/libgccjit/pull/6
Something in this context broke build: ./tm.texi:11390: warning: node next `Rust Language and ABI' in menu `Named Address Spaces' and in sectioning `JIT Language and ABI' differ ./tm.texi:11407: warning: node `Named Address Spaces' is next for `JIT Language and ABI' in sectioning but not in menu ./tm.texi:11407: warning: node `Rust Language and ABI' is prev for `JIT Language and ABI' in sectioning but not in menu ./tm.texi:11407: warning: node `Target Macros' is up for `JIT Language and ABI' in sectioning but not in menu ./tm.texi:5: node `Target Macros' lacks menu item for `JIT Language and ABI' despite being its Up target ./tm.texi:11418: warning: node prev `Named Address Spaces' in menu `Rust Language and ABI' and in sectioning `JIT Language and ABI' differ make[1]: *** [Makefile:3845: doc/gccint.info] Error 1 > Thanks. > > Le 29/10/2024 à 17:04, David Malcolm a écrit : > > On Tue, 2024-10-29 at 07:59 -0400, Antoni Boucher wrote: > >> David: Arthur reviewed the gccrs patch and would be OK with it. > >> > >> Could you please take a look and review it? > > > > https://github.com/Rust-GCC/gccrs/pull/3195 > > looks good to me; thanks! > > > > Dave > > > >> > >> Le 2024-10-17 à 11 h 38, Antoni Boucher a écrit : > >>> Hi. > >>> Thanks for the review, David! > >>> > >>> I talked to Arthur and he's OK with having a file to include in > >>> both > >>> gccrs and libgccjit. > >>> > >>> I sent the patch to gccrs to move the code in a new file that we > >>> can > >>> include in both frontends: > >>> https://github.com/Rust-GCC/gccrs/pull/3195 > >>> > >>> I also renamed gcc_jit_target_info_supports_128bit_int to > >>> gcc_jit_target_info_supports_target_dependent_type because a > >>> subsequent > >>> patch will allow to check if other types are supported like > >>> _Float16 and > >>> _Float128. > >>> > >>> Here's the patch for libgccjit updated to include this file. > >>> > >>> Thanks. > >>> > >>> Le 2024-06-26 à 17 h 55, David Malcolm a écrit : > >>>> On Sun, 2024-03-10 at 12:05 +0100, Iain Buclaw wrote: > >>>>> Excerpts from David Malcolm's message of März 5, 2024 4:09 pm: > >>>>>> On Thu, 2023-11-09 at 19:33 -0500, Antoni Boucher wrote: > >>>>>>> Hi. > >>>>>>> See answers below. > >>>>>>> > >>>>>>> On Thu, 2023-11-09 at 18:04 -0500, David Malcolm wrote: > >>>>>>>> On Thu, 2023-11-09 at 17:27 -0500, Antoni Boucher wrote: > >>>>>>>>> Hi. > >>>>>>>>> This patch adds support for getting the CPU features in > >>>>>>>>> libgccjit > >>>>>>>>> (bug > >>>>>>>>> 112466) > >>>>>>>>> > >>>>>>>>> There's a TODO in the test: > >>>>>>>>> I'm not sure how to test that gcc_jit_target_info_arch > >>>>>>>>> returns > >>>>>>>>> the > >>>>>>>>> correct value since it is dependant on the CPU. > >>>>>>>>> Any idea on how to improve this? > >>>>>>>>> > >>>>>>>>> Also, I created a CStringHash to be able to have a > >>>>>>>>> std::unordered_set<const char *>. Is there any built-in > >>>>>>>>> way > >>>>>>>>> of > >>>>>>>>> doing > >>>>>>>>> this? > >>>>>>>> > >>>>>>>> Thanks for the patch. > >>>>>>>> > >>>>>>>> Some high-level questions: > >>>>>>>> > >>>>>>>> Is this specifically about detecting capabilities of the > >>>>>>>> host > >>>>>>>> that > >>>>>>>> libgccjit is currently running on? or how the target was > >>>>>>>> configured > >>>>>>>> when libgccjit was built? > >>>>>>> > >>>>>>> I'm less sure about this part. I'll need to do more tests. > >>>>>>> > >>>>>>>> > >>>>>>>> One of the benefits of libgccjit is that, in theory, we > >>>>>>>> support > >>>>>>>> all > >>>>>>>> of > >>>>>>>> the targets that GCC already supports. Does this patch > >>>>>>>> change > >>>>>>>> that, > >>>>>>>> or > >>>>>>>> is this more about giving client code the ability to > >>>>>>>> determine > >>>>>>>> capabilities of the specific host being compiled for? > >>>>>>> > >>>>>>> This should not change that. If it does, this is a bug. > >>>>>>> > >>>>>>>> > >>>>>>>> I'm nervous about having per-target jit code. Presumably > >>>>>>>> there's a > >>>>>>>> reason that we can't reuse existing target logic here - > >>>>>>>> can you > >>>>>>>> please > >>>>>>>> describe what the problem is. I see that the ChangeLog > >>>>>>>> has: > >>>>>>>> > >>>>>>>>> * config/i386/i386-jit.cc: New file. > >>>>>>>> > >>>>>>>> where i386-jit.cc has almost 200 lines of nontrivial > >>>>>>>> code. > >>>>>>>> Where > >>>>>>>> did > >>>>>>>> this come from? Did you base it on existing code in our > >>>>>>>> source > >>>>>>>> tree, > >>>>>>>> making modifications to fit the new internal API, or did > >>>>>>>> you > >>>>>>>> write > >>>>>>>> it > >>>>>>>> from scratch? In either case, how onerous would this be > >>>>>>>> for > >>>>>>>> other > >>>>>>>> targets? > >>>>>>> > >>>>>>> This was mostly copied from the same code done for the Rust > >>>>>>> and D > >>>>>>> frontends. > >>>>>>> See this commit and the following: > >>>>>>> https://gcc.gnu.org/git/? > >>>>>>> p=gcc.git;a=commit;h=b1c06fd9723453dd2b2ec306684cb806dc2b4f > >>>>>>> bb > >>>>>>> The equivalent to i386-jit.cc is there: > >>>>>>> https://gcc.gnu.org/git/? > >>>>>>> p=gcc.git;a=commit;h=22e3557e2d52f129f2bbfdc98688b945dba28d > >>>>>>> c9 > >>>>>> > >>>>>> [CCing Iain and Arthur re those patches; for reference, the > >>>>>> patch > >>>>>> being > >>>>>> discussed is attached to : > >>>>>> https://gcc.gnu.org/pipermail/jit/2024q1/001792.html ] > >>>>>> > >>>>>> One of my concerns about this patch is that we seem to be > >>>>>> gaining > >>>>>> code > >>>>>> that's per-(frontend x config) which seems to be copied and > >>>>>> pasted > >>>>>> with > >>>>>> a search and replace, which could lead to an M*N explosion. > >>>>>> > >>>>> > >>>>> That's certainly the case with the configure/make rules. Itself > >>>>> I > >>>>> think > >>>>> is copied originally from the {cpu_type}-protos.h machinery. > >>>>> > >>>>> It might be worth pointing out that the c-family of front-ends > >>>>> don't > >>>>> have separate headers because their per-target macros are > >>>>> defined in > >>>>> {cpu_type}.h directly - for better or worse. > >>>>> > >>>>>> Is there any real difference between the per-config code for > >>>>>> the > >>>>>> different frontends, or should there be a general "enumerate > >>>>>> all > >>>>>> features of the target" hook that's independent of the > >>>>>> frontend? > >>>>>> (but > >>>>>> perhaps calls into it). > >>>>>> > >>>>> > >>>>> As far as I understand, the configure parts should all be > >>>>> identical > >>>>> between tm_p, tm_d, tm_rust, ..., so would benefit from being > >>>>> templated > >>>>> to aid any other front-ends adding in their own per target > >>>>> hooks. > >>>>> > >>>>>> Am I right in thinking that (rustc with default LLVM backend) > >>>>>> has > >>>>>> some > >>>>>> set of feature strings that both (rustc with > >>>>>> rustc_codegen_gcc) and > >>>>>> gccrs are trying to emulate? If so, is it presumably a goal > >>>>>> that > >>>>>> libgccjit gives identical results to gccrs? If so, would it > >>>>>> be > >>>>>> crazy > >>>>>> for libgccjit to consume e.g. config/i386/i386-rust.cc ? > >>>>> > >>>>> I don't know whether libgccjit can just pull in directly the > >>>>> implementation of the rust target hooks here. > >>>> > >>>> Sorry for the delay in responding. > >>>> > >>>> I don't want to be in the business of maintaining a copy of the > >>>> per- > >>>> target code for "jit", and I think it makes sense for libgccjit > >>>> to > >>>> return identical information compared to gccrs. > >>>> > >>>> So I think it would be ideal for jit to share code with rust for > >>>> this, > >>>> rather than do a one-time copy-and-paste followed by a ongoing > >>>> "keep > >>>> things updated" treadmill. > >>>> > >>>> Presumably there would be Makefile.in issues given that e.g. > >>>> Makefile > >>>> has i386-rust.o listed in: > >>>> > >>>> # Target specific, Rust specific object file > >>>> RUST_TARGET_OBJS= i386-rust.o linux-rust.o > >>>> > >>>> One approach might be to move e.g. the i386-rust.cc code into, > >>>> say, a > >>>> i386-rust-and-jit.inc file and #include it from i386-rust.cc and > >>>> i386- > >>>> jit.cc (with a similar approach for other targets) > >>>> > >>>> Antoni, Arthur, would you each be OK with that? > >>>> > >>>> > >>>>> The per-frontend target > >>>>> hooks usually also make use of code specific to that front-end > >>>>> - > >>>>> TARGET_CPU_CPP_BUILTINS and others can't be used by a non-c- > >>>>> family > >>>>> front-end without adding a plethora of stubs, for example. > >>>>> > >>>>> Whether or not libgccjit wants to give identical information as > >>>>> as > >>>>> rust > >>>>> I think is a decision for you as the maintainer of its API. > >>>> > >>>> Also a question for Antoni and Arthur: is that desirable for the > >>>> two > >>>> gcc rust approaches? > >>>> > >>>> Thanks > >>>> Dave > >> > > >
