David: Ping.
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?
>
> 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
>