Skip to content

Conversation

@thecppzoo
Copy link
Owner

Reviewers: please, in the case you're satisfied with the PR up to a certain commit, please state your satisfaction in a comment. If the other reviewer is also satisfied as of the same commit as you, then please merge up to that commit to the master branch. This is more "agile"

set_target_properties(${TARGET_NAME} PROPERTIES
XCODE_ATTRIBUTE_ENABLE_AVX YES
XCODE_ATTRIBUTE_ENABLE_AVX2 YES
XCODE_ATTRIBUTE_OTHER_CPLUSPLUSFLAGS "-mavx -mavx2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

will run this on my M1 machine later on today, but i think you'll need another layer here of

if (${TARGET_PLATFORM} == X86_64) 
    // enable avx
endif()

will find the code i've used before which will do this only when AVX is actually detected on the machine, but i think this will need to be updated to work on ARM

Copy link
Owner Author

Choose a reason for hiding this comment

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

Wait, doesn't XCode in M1 allow you to cross compile transparently to x86-64?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I see you're assuming it will make the dual ARM+Intel binary ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be cool to try and keep the code (where possible) architecture independent, otherwise could become a pain to try and go back and wrangle it to work in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Keeping code as arch independent as possible seems critical if our objectives are to work everywhere. Which I think they are.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, friends, architecture independence is a primary objective. But we have the freedom to draft and experiment architecture specific.

@Scottbruceheart
Copy link
Collaborator

Did I overlook the tests for the added functions that act as evidence that the benchmarks are doing the right thing?

The GCC strlen is painfully hard to read. I much prefer our methods.

SWAR atoi comment doesn't read easily to me (I had to go reread Lemire's). Needs a rework.

If we're intending this as demo code, that directly means we want other SWE to read and understand and believe correctness and be impressed with as little cognitive effort on their part as possible.

@thecppzoo
Copy link
Owner Author

Did I overlook the tests for the added functions that act as evidence that the benchmarks are doing the right thing?

Yes, as of this commit, in catch2swar-demos.cpp:
https://github.com/thecppzoo/zoo/pull/69/files#diff-286f2a6168f774ab1f7b54d49d8549b8c721a01199a226197f6a752573790674

SWAR atoi comment doesn't read easily to me (I had to go reread Lemire's). Needs a rework.

If we're intending this as demo code, that directly means we want other SWE to read and understand and believe correctness and be impressed with as little cognitive effort on their part as possible.

This is not advertising nor marketing. These demos are the necessary complement to the code to avoid frustrations of not being able to see things being used and to provide us feedback to drive further development.

What is the comment that doesn't read easily?

@thecppzoo
Copy link
Owner Author

The GCC strlen is painfully hard to read. I much prefer our methods.

Thanks, but your comment is entirely subjective.

What is the lesson to be learned from GCC's code, or from "our methods"? GCC's code had a programming technique lurking in it that took me three attempts in three years to fully grok. It has several "low-hanging-fruit" performance mistakes, and our code implicitly showcases the superiority of little-endian encoding for this type of task.

I think that this deserves a whole essay to discuss in proper detail and context, it would be helpful to know your opinion with full articulation of its points.

@Scottbruceheart Scottbruceheart merged commit f129574 into master Feb 11, 2024
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.

4 participants