Conversation
| while (wordIndex + cycleLen <= fullWordCount) | ||
| { | ||
| size_t idx = 0; | ||
| while (idx + 8 <= cycleLen) | ||
| { |
There was a problem hiding this comment.
This question applies to all main and nested while loops in this section of code: is there a particular reason for using while instead of for?
I know this is dangerously close to a coding style comment and I'm the first to say we shouldn't spend too much time on those, certainly in this project. However, I think combining the loop condition and the increment expression (which is neatly at the end of every loop) in one line clarifies the "cycle logic" in this particular case.
|
Good point - the structure required it the way I was doing the preprocessor stuff originally, but now it looks silly. Any cases that have init, condition, AND increment should be a for !
Thanks
Dave
… On Oct 26, 2025, at 6:27 AM, Rutger van Bergen ***@***.***> wrote:
@rbergen commented on this pull request.
In PrimeCPP/solution_5/PrimeCPP_array.cpp <#1024 (comment)>:
> + while (wordIndex + cycleLen <= fullWordCount)
+ {
+ size_t idx = 0;
+ while (idx + 8 <= cycleLen)
+ {
This question applies to all main and nested while loops in this section of code: is there a particular reason for using while instead of for?
I know this is dangerously close to a coding style comment and I'm the first to say we shouldn't spend too much time on those, certainly in this project. However, I think combining the loop condition and the increment expression (which is neatly at the end of every loop) in one line clarifies the "cycle logic" in this particular case.
—
Reply to this email directly, view it on GitHub <#1024 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4HCFY357O2RCD4CRVG5JD3ZSVZHAVCNFSM6AAAAACKGMZ53OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGOBRGA2DCMBXGA>.
You are receiving this because you authored the thread.
|
|
So I went through the code and did an AI refactor to for(), and it’s uglier :-). Most of the fors() can’t have the indexer in score because it’s used in multiple loops.
You wind up with a lot of this, and I actually prefer the while()!
- Dave

… On Oct 26, 2025, at 6:27 AM, Rutger van Bergen ***@***.***> wrote:
@rbergen commented on this pull request.
In PrimeCPP/solution_5/PrimeCPP_array.cpp <#1024 (comment)>:
> + while (wordIndex + cycleLen <= fullWordCount)
+ {
+ size_t idx = 0;
+ while (idx + 8 <= cycleLen)
+ {
This question applies to all main and nested while loops in this section of code: is there a particular reason for using while instead of for?
I know this is dangerously close to a coding style comment and I'm the first to say we shouldn't spend too much time on those, certainly in this project. However, I think combining the loop condition and the increment expression (which is neatly at the end of every loop) in one line clarifies the "cycle logic" in this particular case.
—
Reply to this email directly, view it on GitHub <#1024 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4HCFY357O2RCD4CRVG5JD3ZSVZHAVCNFSM6AAAAACKGMZ53OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGOBRGA2DCMBXGA>.
You are receiving this because you authored the thread.
|
|
The screen snippet doesn't come through when you email respond to GitHub comments, but I imagine it's an example of a for loop with an empty first clause ( If that's right then I was aware of this, but I still think the looping logic itself is more clearly expressed in a |
|
I prefer the current while structure! So, LGTM*
- Dave
* let’s get this merged!
… On Oct 26, 2025, at 10:49 AM, Rutger van Bergen ***@***.***> wrote:
rbergen
left a comment
(PlummersSoftwareLLC/Primes#1024)
<#1024 (comment)>
The screen snippet doesn't come through when you email respond to GitHub comments, but I imagine it's an example of a for loop with an empty first clause (for (; wordIndex + cycleLen <= fullWordCount; wordIdex += cycleLen) or for(; idx + 8 <= cycleLen; idx += 8)).
If that's right then I was aware of this, but I still think the looping logic itself is more clearly expressed in a for context. However, the fact I'm one of the maintainers of this project doesn't change it's your solution, so if you prefer the while, then that's what we'll merge.
—
Reply to this email directly, view it on GitHub <#1024 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4HCF36UCQX7RKOBCTIFU33ZT3SJAVCNFSM6AAAAACKGMZ53OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTINBYG4ZDQOJUGY>.
You are receiving this because you authored the thread.
|
This adds vectorization approaches for SSE2, AVX, AVX512, and NEON. It's worth about 30% perf on the Mac and ubdellamd.
It also enables restrict in the hopes it keeps the compiler assured that sieve memory is not being aliased.
drag-raceas the target branch.