search

DrTimothyAldenDavis / GraphBLAS

SuiteSparse:GraphBLAS: graph algorithms in the language of linear algebra. For production: (default) STABLE branch. Code development: ask me for the right branch before submitting a PR. video intro: https://youtu.be/Tj5y6d7FegI .
http://faculty.cse.tamu.edu/davis/GraphBLAS.html
Other
345 stars 61 forks source link

xxhash.h: Slightly overhaul rules for definition of `XXH_ALIGN`. #225

Closed mmuetzel closed 10 months ago

mmuetzel commented 12 months ago

Use special case for MSVC only in affected versions of that compiler. Move definition for C++ code above the one for C code.

See also the comments in https://github.com/DrTimothyAldenDavis/SuiteSparse/issues/315 starting at https://github.com/DrTimothyAldenDavis/SuiteSparse/issues/315#issuecomment-1613491207.

sdirkse67 commented 12 months ago

Seems reasonable to limit as much as possible the use of special-case code that assumes a broken MSVC compiler, and to assume/hope that MSVC will stay fixed.

mmuetzel commented 10 months ago

@DrTimothyAldenDavis: I hope you're feeling better.

This is a fairly localized change. Would it be ok to merge this?

sdirkse67 commented 10 months ago

In case you are asking me: yes, I would approve this merge.

DrTimothyAldenDavis commented 10 months ago

Changing the branch to the current draft of GraphBLAS v8.2.0, which is also the current version in the development branch of SuiteSparse/GraphBLAS.

I'll give this a try here and in the dev2 branch of SuiteSparse. I can't really test this update in the GraphBLAS github repo since it requires a CI on Windows. That's in the SuiteSparse repo, not here. So I'll merge this change into the v8.2.0 branch and then copy the change to dev2 of SuiteSparse.

mmuetzel commented 10 months ago

Thank you for merging. I'm unsure where to open PRs for GraphBLAS. Here or in the SuiteSparse repository?

DrTimothyAldenDavis commented 10 months ago

Good question. In the normal case, my "master" branch of GraphBLAS is my primary development branch of GraphBLAS. Once that becomes stable, I do a formal release and then copy the stable release to SuiteSparse/GraphBLAS in the SuiteSparse git hub repo (the dev2 branch).

However, since the SuiteSparse build system is also changing (thanks to your input), I'm also working on SuiteSparse itself, and thus also SuiteSparse/GraphBLAS. So any changes to the dev2 branch of SuiteSparse/GraphBLAS would also get copied into the master branch of GraphBLAS.

But I'm not using the master branch of GraphBLAS because of the upcoming v2.1 C API release.

PRs for GraphBLAS would now go in two places, since I'm working on 2 versions of GraphBLAS at the same time. So it's a bit complicated.

The GraphBLAS v8.2.0 branch is tracking the dev2 branch of SuiteSparse / GraphBLAS. That's the next planned stable release with all the cmake build system changes you've made to SuiteSparse. So v8.2.0 of GraphBLAS is identical to the dev2 SuiteSparse/GraphBLAS folder. You can do a PR to either place and it's essentially the same thing.

Finally, I'm also working on GraphBLAS v9.0.0, in the GrB_get_set_proposal branch of the GraphBLAS github. That version is being updated based on the GraphBLAS v2.1 C API changes, here: https://github.com/GraphBLAS/graphblas-api-c/tree/GrB_get_set . I can't release that yet since the v2.1 C API isn't stable yet. It soon will be released in a few weeks, however.

So I'm essentially keeping track of 3 active branches of GraphBLAS:

(1) v8.2.0 in the GraphBLAS repo (2) dev2 in SuiteSparse/GraphBLAS folder of the GraphBLAS repo (identical to (1)). (3) GrB_get_set_proposal branch of the GraphBLAS repo, which differs from (1) and (2) because of all the new GrB_get/set methods. But this version is also being updated with the same changes to the build system in (1) and (2).

Juggling too many things here, I know ...