Skip to content

Conversation

r-devulap
Copy link
Member

  • Add new file avx512fp16-16bit-qsort.hpp
  • Add new tests for _Float16 on SPR
  • Update meson and Make build

Copy link

@thac0 thac0 left a comment

Choose a reason for hiding this comment

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

Some comments from the peanut gallery. 😁

If you'd like, I could post a companion PR, if you'd like to see how I'd do the meson coding... but only if you're interested.

@@ -1,16 +1,15 @@
CXX ?= g++
CXX = g++-12
Copy link

Choose a reason for hiding this comment

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

That'll bite if others don't use the exact same distro and packaging as you. Best not to hard-assign the compiler in a Makefile.

Continue to use ?= so the end-user can override (with, say, g++-12) in an env var.

Copy link
Member Author

Choose a reason for hiding this comment

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

-march=sapphirerapids is supported only with g++-12 and above and adding conditional compile in Make is non trivial, at least to me :) The main build system is Meson anyway which handles this relatively easy.

Copy link

@thac0 thac0 Mar 15, 2023

Choose a reason for hiding this comment

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

Not just you 😁 . "Read the docs, provide path to compiler that support the following flags" is about all you can do with a custom Makefile.

Comment on lines +5 to +8
src = include_directories('src')
bench = include_directories('benchmarks')
utils = include_directories('utils')
tests = include_directories('tests')
Copy link

Choose a reason for hiding this comment

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

These can all be assigned to one variable, using a single include_directories() invocation with an array.

@r-devulap r-devulap merged commit dc57afb into numpy:main Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants