Hi,
On 3/5/24 16:09, David Malcolm wrote:
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. 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=b1c06fd9723453dd2b2ec306684cb806dc2b4fbb
The equivalent to i386-jit.cc is there:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=22e3557e2d52f129f2bbfdc98688b945dba28dc9
[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.
I think this is definitely already the case, and it would be worth
investigating if C/C++/Rust/jit can reuse a similar set of target files,
or how to factor them together. I imagine that all of these components
share similar needs for the targets they support.
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).
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 think this would definitely make sense, and it could probably be
extended to other frontends. For the time being I think it makes sense
to try it out for gccrs and jit. But finding a fitting name will be hard :)
Best,
Arthur
Dave
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?
I will do.
[...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?
It's the march string, like "znver2", for instance.
It remains valid until we free the gcc_jit_target_info object.
Thanks again; hope the above makes sense
Dave
OpenPGP_0x1B3465B044AD9C65.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
