Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cpu features detection #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Cpu features detection #6

wants to merge 2 commits into from

Conversation

antoyo
Copy link
Owner

@antoyo antoyo commented Oct 16, 2024 •
edited
Loading

Outdated patch on ML: https://gcc.gnu.org/pipermail/jit/2023q4/001696.html

  • Rename gcc_jit_target_info_supports_128bit_int to the name used later for floats, possibly gcc_jit_target_info_supports_type.
  • Merge gccrs patch.
  • Send updated patch with the gccrs refactor on the ML.
  • Remove linux-jit since it's currently not used?

The first commit should be merged via gccrs here.

@antoyo
Reorganize the CPU feature detection
0d1c077 
Move the code from i386-rust.cc to i386-rust-and-jit.inc so that it can
be reused by libgccjit.
@antoyo antoyo force-pushed the cpu-features-detection branch from 3ba6353 to ec10dc2 Compare October 16, 2024 21:11
@antoyo antoyo added the waiting for review Waiting for review from upstream libgccjit maintainer label Oct 17, 2024
@antoyo antoyo force-pushed the cpu-features-detection branch from ec10dc2 to 033c79b Compare October 17, 2024 15:28
davidmalcolm
davidmalcolm requested changes Nov 1, 2024
View reviewed changes
Copy link
Collaborator

@davidmalcolm davidmalcolm left a comment •
edited by antoyo
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I like the overall approach.

  • - Please add documentation for the new API entrypoints.
  • - See the notes about direct inclusion of C++ standard headers

gcc/jit/jit-target.h

#include <string>
#include <unordered_map>
#include <unordered_set>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't usually include C++ stdlib headers directly like this (due to problems with macros, IIRC). See gcc/system.h which has e.g.:

#ifdef INCLUDE_STRING
# include <string>
#endif

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added INCLUDE_STRING, but it seems there are none for the unordered maps and sets.

gcc/jit/jit-target.cc Outdated Show resolved Hide resolved
@antoyo antoyo added waiting on author and removed waiting for review Waiting for review from upstream libgccjit maintainer labels Nov 1, 2024
@antoyo antoyo force-pushed the cpu-features-detection branch from 033c79b to 212731f Compare November 2, 2024 00:06
antoyo
antoyo commented Nov 2, 2024
View reviewed changes
Copy link
Owner Author

@antoyo antoyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidmalcolm: I added the doc, fixed the include and the formatting.
Thanks for the review.

gcc/testsuite/jit.dg/test-target-info.c
CHECK_VALUE (sse2_supported, 1);

const char *arch = gcc_jit_target_info_arch (info);
// TODO: not sure what value to check here.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidmalcolm: Do you have any idea on how I could test this?

gcc/config.gcc
@@ -381,6 +389,7 @@ arm*-*-*)
c_target_objs="arm-c.o"
cxx_target_objs="arm-c.o"
d_target_objs="arm-d.o"
#jit_target_objs="arm-jit.o"
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidmalcolm: Should I remove all the commented lines like this one?

gcc/jit/jit-target.h

#include <string>
#include <unordered_map>
#include <unordered_set>
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added INCLUDE_STRING, but it seems there are none for the unordered maps and sets.

gcc/config/linux-jit.cc
#include "jit/jit-target.h"
#include "jit/jit-target-def.h"

// TODO: remove this hook?
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidmalcolm: Should I remove this file since it's not currently used?

gcc/jit/jit-target.h
Comment on lines +32 to +51
static size_t hash_cstr (const char *s)
{
const size_t seed = 0;
return std::_Hash_bytes (s, std::strlen (s), seed);
}

struct CStringHash {
size_t operator() (const char* const &string) const {
auto res = hash_cstr (string);
return res;
}
};

struct CStringEqual {
bool
operator() (const char *const &string1, const char *const &string2) const
{
return strcmp (string1, string2) == 0;
}
};
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidmalcolm: Do you know if there's a better way to do that?

@antoyo antoyo added waiting for review Waiting for review from upstream libgccjit maintainer and removed waiting on author labels Nov 2, 2024
@antoyo
libgccjit: Add ability to get CPU features
7c40087 
gcc/ChangeLog:
	PR jit/112466
	* Makefile.in (tm_jit_file_list, tm_jit_include_list, TM_JIT_H,
	JIT_TARGET_DEF, JIT_TARGET_H, JIT_TARGET_OBJS): New variables.
	(tm_jit.h, cs-tm_jit.h, jit/jit-target-hooks-def.h,
	s-jit-target-hooks-def-h, default-jit.o): New rules.
	(s-tm-texi): Also check timestamp on jit-target.def.
	(generated_files): Add TM_JIT_H and jit/jit-target-hooks-def.h.
	(build/genhooks.o): Also depend on JIT_TARGET_DEF.
	* config.gcc (tm_jit_file, jit_target_objs, target_has_targetjitm):
	New variables.
	* config/i386/t-i386 (i386-jit.o): New rule.
	* config/t-linux (linux-jit.o): New rule.
	* configure: Regenerate.
	* configure.ac (tm_jit_file_list, tm_jit_include_list,
	jit_target_objs): Add substitutes.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in (targetjitm): Document.
	(target_has_targetjitm): Document.
	* genhooks.cc: Include jit/jit-target.def.
	* config/default-jit.cc: New file.
	* config/i386/i386-jit.cc: New file.
	* config/i386/i386-jit.h: New file.
	* config/linux-jit.cc: New file.

gcc/jit/ChangeLog:
	PR jit/112466
	* Make-lang.in (JIT_OBJS): New variable.
	* jit-playback.cc (replay): Include jit-target.h and initialize
	target.
	* jit-playback.h (class get_target_info): New class.
	* jit-recording.cc (recording::context::get_target_info): New
	method.
	* jit-recording.h (recording::context::get_target_info): New
	method.
	* libgccjit.cc: Include jit-target.h.
	(struct gcc_jit_target_info): New struct.
	(gcc_jit_context_get_target_info, gcc_jit_target_info_release,
	gcc_jit_target_info_cpu_supports, gcc_jit_target_info_arch,
	gcc_jit_target_info_supports_target_dependent_type): New functions.
	* libgccjit.h (gcc_jit_context_get_target_info,
	gcc_jit_target_info_release, gcc_jit_target_info_cpu_supports,
	gcc_jit_target_info_arch,
	gcc_jit_target_info_supports_target_dependent_type):
	New functions.
	* libgccjit.map (LIBGCCJIT_ABI_29): New ABI tag.
	* docs/topics/compilation.rst: Add documentation for the
	functions gcc_jit_context_get_target_info, gcc_jit_target_info_release,
	gcc_jit_target_info_cpu_supports, gcc_jit_target_info_arch,
	gcc_jit_target_info_supports_target_dependent_type.
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_29): New ABI tag.
	* jit-target-def.h: New file.
	* jit-target.cc: New file.
	* jit-target.def: New file.
	* jit-target.h: New file.

gcc/testsuite/ChangeLog:
	PR jit/112466
	* jit.dg/all-non-failing-tests.h: Mention
	test-target-info.c.
	* jit.dg/test-target-info.c: New test.
@antoyo antoyo force-pushed the cpu-features-detection branch from 212731f to 7c40087 Compare November 2, 2024 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidmalcolm davidmalcolm davidmalcolm requested changes

Assignees
No one assigned
Labels
waiting for review Waiting for review from upstream libgccjit maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@antoyo @davidmalcolm