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? 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? 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? I'm not at expert at target hooks (or at the i386 backend), so if we do go with this approach I'd want someone else to review those parts of the patch. Have you verified that GCC builds with this patch with jit *not* enabled in the enabled languages? [...snip...] A nitpick: > +.. function:: const char * \ > + gcc_jit_target_info_arch (gcc_jit_target_info *info) > + > + Get the architecture of the currently running CPU. What does this string look like? How long does the pointer remain valid? Thanks again; hope the above makes sense Dave
