convert most of core to header only #2138

boxerab · 2024-05-04T20:50:40Z

if per_target.cc becomes header only, then library cannot be built as there are no longer any .cc files

Here is the error:

CMake Error: Cannot determine link language for target "hwy".
CMake Error: CMake can not determine linker language for target: hwy
CMake Error in CMakeLists.txt:
  Exporting the target "hwy" is not allowed since its linker language cannot
  be determined

Even with current commit, we get errors in tests:



/home//src/highway/hwy/tests/mask_mem_test.cc:35:8: error: redefinition of ‘struct hwy::N_EMU128::TestMaskedLoad’
   35 | struct TestMaskedLoad {
      |        ^~~~~~~~~~~~~~
In file included from /home//src/highway/hwy/foreach_target.h:316,
                 from /home//src/highway/hwy/tests/mask_mem_test.cc:27:
/home//src/highway/hwy/tests/mask_mem_test.cc:35:8: note: previous definition of ‘struct hwy::N_EMU128::TestMaskedLoad’
   35 | struct TestMaskedLoad {
      |        ^~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:69:19: error: redefinition of ‘void hwy::N_EMU128::TestAllMaskedLoad()’
   69 | HWY_NOINLINE void TestAllMaskedLoad() {
      |                   ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:69:19: note: ‘void hwy::N_EMU128::TestAllMaskedLoad()’ previously defined here
   69 | HWY_NOINLINE void TestAllMaskedLoad() {
      |                   ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:73:8: error: redefinition of ‘struct hwy::N_EMU128::TestMaskedScatter’
   73 | struct TestMaskedScatter {
      |        ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:73:8: note: previous definition of ‘struct hwy::N_EMU128::TestMaskedScatter’
   73 | struct TestMaskedScatter {
      |        ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:111:19: error: redefinition of ‘void hwy::N_EMU128::TestAllMaskedScatter()’
  111 | HWY_NOINLINE void TestAllMaskedScatter() {
      |                   ^~~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:111:19: note: ‘void hwy::N_EMU128::TestAllMaskedScatter()’ previously defined here
  111 | HWY_NOINLINE void TestAllMaskedScatter() {
      |                   ^~~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:115:8: error: redefinition of ‘struct hwy::N_EMU128::TestScatterIndexN’
  115 | struct TestScatterIndexN {
      |        ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:115:8: note: previous definition of ‘struct hwy::N_EMU128::TestScatterIndexN’
  115 | struct TestScatterIndexN {
      |        ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:167:19: error: redefinition of ‘void hwy::N_EMU128::TestAllScatterIndexN()’
  167 | HWY_NOINLINE void TestAllScatterIndexN() {
      |                   ^~~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:167:19: note: ‘void hwy::N_EMU128::TestAllScatterIndexN()’ previously defined here
  167 | HWY_NOINLINE void TestAllScatterIndexN() {
      |                   ^~~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:171:8: error: redefinition of ‘struct hwy::N_EMU128::TestMaskedGather’
  171 | struct TestMaskedGather {
      |        ^~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:171:8: note: previous definition of ‘struct hwy::N_EMU128::TestMaskedGather’
  171 | struct TestMaskedGather {
      |        ^~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:210:19: error: redefinition of ‘void hwy::N_EMU128::TestAllMaskedGather()’
  210 | HWY_NOINLINE void TestAllMaskedGather() {
      |                   ^~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:210:19: note: ‘void hwy::N_EMU128::TestAllMaskedGather()’ previously defined here
  210 | HWY_NOINLINE void TestAllMaskedGather() {
      |                   ^~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:214:8: error: redefinition of ‘struct hwy::N_EMU128::TestGatherIndexN’
  214 | struct TestGatherIndexN {
      |        ^~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:214:8: note: previous definition of ‘struct hwy::N_EMU128::TestGatherIndexN’
  214 | struct TestGatherIndexN {
      |        ^~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:258:19: error: redefinition of ‘void hwy::N_EMU128::TestAllGatherIndexN()’
  258 | HWY_NOINLINE void TestAllGatherIndexN() {
      |                   ^~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:258:19: note: ‘void hwy::N_EMU128::TestAllGatherIndexN()’ previously defined here
  258 | HWY_NOINLINE void TestAllGatherIndexN() {
      |                   ^~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:262:8: error: redefinition of ‘struct hwy::N_EMU128::TestBlendedStore’
  262 | struct TestBlendedStore {
      |        ^~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:262:8: note: previous definition of ‘struct hwy::N_EMU128::TestBlendedStore’
  262 | struct TestBlendedStore {
      |        ^~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:293:19: error: redefinition of ‘void hwy::N_EMU128::TestAllBlendedStore()’
  293 | HWY_NOINLINE void TestAllBlendedStore() {
      |                   ^~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:293:19: note: ‘void hwy::N_EMU128::TestAllBlendedStore()’ previously defined here
  293 | HWY_NOINLINE void TestAllBlendedStore() {
      |                   ^~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:297:7: error: redefinition of ‘class hwy::N_EMU128::TestStoreMaskBits’
  297 | class TestStoreMaskBits {
      |       ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:297:7: note: previous definition of ‘class hwy::N_EMU128::TestStoreMaskBits’
  297 | class TestStoreMaskBits {
      |       ^~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:377:19: error: redefinition of ‘void hwy::N_EMU128::TestAllStoreMaskBits()’
  377 | HWY_NOINLINE void TestAllStoreMaskBits() {
      |                   ^~~~~~~~~~~~~~~~~~~~
/home//src/highway/hwy/tests/mask_mem_test.cc:377:19: note: ‘void hwy::N_EMU128::TestAllStoreMaskBits()’ previously defined here
  377 | HWY_NOINLINE void TestAllStoreMaskBits() {
      |                   ^~~~~~~~~~~~~~~~~~~~
In file included from /home//src/highway/hwy/targets.cc:23,
                 from /home//src/highway/hwy/targets.h:345,
                 from /home//src/highway/hwy/highway.h:22,
                 from /home//src/highway/hwy/timer-inl.h:26,
                 from /home//src/highway/hwy/timer.cc:25,
                 from /home//src/highway/hwy/timer.h:67,
                 from /home//src/highway/hwy/nanobenchmark.h:52,
                 from /home//src/highway/hwy/tests/mask_mem_test.cc:23:
/home//src/highway/hwy/highway.h:226:38: error: ‘N_SSSE3’ has not been declared

CMakeLists.txt

jan-wassenberg · 2024-05-06T14:15:06Z

hwy/abort.cc

@@ -20,6 +20,10 @@
 namespace hwy {

 namespace {
+
+#ifdef HWY_HEADER_ONLY
+inline


I think for header files, we want static inline just to be sure/safe.

Doesn't anonymous namespace take the place of static in this case ?

hm. Now that you mention it, if we want to support header-only, then we should avoid anon namespace, even in .cc files, because those will also be included in the single header.
The reason is that multiple includes of the single header from different TUs would seem to violate the One Definition Rule.

Probably a good idea, but I don't think that's issue with the compile error. Compile currently chokes on
mask_slide_test.cc. so it is able to compile a number of other tests successfully. When you have a chance, take a look as I can't make head or tail of the error.

Interesting. What's the error? I'm not seeing this in the build logs, currently they are failing at CMake already due to the CXX project setting.

Thanks! I think the problem is that this test happens to have the same name as another test. Will fix shortly.

Unfortunately, error persists with this fix.

hm, that's mystifying. Is that happening for all of the struct/class in this file?
TestStoreMaskBits is the only class, not that that should make a difference.

That class name is not repeated in other files. I'm not seeing anything different about this file compared to others.

Thanks, indeed it is strange, there must be something else about this test that is different from all the others. Can you try on your system with the header only flag enabled ?

I've currently got lots going on. Would anyone else be interested in having a look?

CMakeLists.txt

PiperOrigin-RevId: 631451998

PiperOrigin-RevId: 631475295

boxerab force-pushed the header_only branch 3 times, most recently from e5b005c to be8573d Compare May 5, 2024 00:53

jan-wassenberg requested changes May 6, 2024

View reviewed changes

jan-wassenberg reviewed May 6, 2024

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

boxerab force-pushed the header_only branch from be8573d to abfa2bb Compare May 6, 2024 18:59

copybara-service bot pushed a commit that referenced this pull request May 7, 2024

fix duplicated test name, refs #2138

4b6009a

PiperOrigin-RevId: 631451998

copybara-service bot mentioned this pull request May 7, 2024

fix duplicated test name, refs #2138 #2144

Closed

copybara-service bot pushed a commit that referenced this pull request May 7, 2024

fix duplicated test name, refs #2138

949c6c8

PiperOrigin-RevId: 631451998

convert most of core to header only

dfa69f9

boxerab force-pushed the header_only branch from abfa2bb to dfa69f9 Compare May 7, 2024 17:23

copybara-service bot pushed a commit that referenced this pull request May 7, 2024

fix duplicated test name, refs #2138

64d8cec

PiperOrigin-RevId: 631451998

copybara-service bot pushed a commit that referenced this pull request May 7, 2024

fix duplicated test name, refs #2138

7430a87

PiperOrigin-RevId: 631475295

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert most of core to header only #2138

convert most of core to header only #2138

boxerab commented May 4, 2024 •

edited

jan-wassenberg May 6, 2024

boxerab May 6, 2024

jan-wassenberg May 6, 2024

boxerab May 6, 2024

jan-wassenberg May 6, 2024

jan-wassenberg May 7, 2024

boxerab May 7, 2024

jan-wassenberg May 8, 2024

boxerab May 8, 2024

jan-wassenberg May 22, 2024

convert most of core to header only #2138

Are you sure you want to change the base?

convert most of core to header only #2138

Conversation

boxerab commented May 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boxerab commented May 4, 2024 •

edited