- User Since
- Oct 6 2017, 3:12 PM (97 w, 1 d)
Sat, Aug 10
Sun, Aug 4
Reminding about this PR.
Sun, Jul 28
Sorry for taking a long time - was busy.
Jun 29 2019
From the comments on the optimiser bugs created I take that this type of code transformation is not feasible in a foreseeable future.
The reason is that this code structure has a non-canonical loop structure and optimiser really likes it's single entry loops.
Jun 22 2019
Unfortunately switch based solution didn't work out - the degradation of 10% is not evenly distributed and, in fact, the results are even significantly worse than current implementation for the case when right hand side.
Jun 19 2019
@DavidBolvansky solved the puzzle!
Reported this as an optimiser bug - maybe they'll have a suggestion: https://bugs.llvm.org/show_bug.cgi?id=42334
Jun 18 2019
Reported switch not being removed as a bug: https://bugs.llvm.org/show_bug.cgi?id=42313 - this should be doable.
After multiple attempts I could not make the "if/switch" based version to produce a similar result.
Since there doesn't seem to be "is_constant_evaluated" in clang, I cannot write a constexpr enabled version of this function.
I can report this as an optimiser bug, if that would be useful.
Jun 11 2019
Jun 10 2019
Thank you, @mclow.lists
Please do not close this revision - I will try to make the optimiser produce the same code with ifs/whiles/switches.
Jun 9 2019
Updated to add a missing '\n'
Not sure who to put as a reviewer.
Dec 17 2018
Dec 15 2018
Dec 14 2018
@ldionne this weekends are an ideal time for me to fix any review comments you might have.
Dec 12 2018
@ldionne - can we please review/merge this? Christmas is coming and I won't have a computer where I can make changes for quite some time.
Dec 10 2018
Dec 9 2018
Recreated the benchmarks in spirit of the sorting benchmarks.
Dec 4 2018
Nov 28 2018
Hi again. Just reminding about this review.
Nov 20 2018
Nov 19 2018
Hi. Just wanted to know what's the status of this change.
Nov 14 2018
Nov 13 2018
Ok - not adding context in git helped.
Which is unpleasant to read though.
Is there a known reason why it doesn't list 'test' in libcxx/algorithms/half_positive.pass.cpp??
Yes, you are right - another artefact of non-apliable patch, thanks.
FYI: updated the description in the test a bit too.
Nov 12 2018
Yes, thank you, missed re-adding the test file.
Nov 11 2018
Recreated the original patch - this was a false alarm as far as I understood it.
I listed everything I've done to tackle and understand the issue in the description of the PR.
Nov 1 2018
Oct 31 2018
FOUND IT! ! difference_type len = _VSTD::distance(first, __last); - should be size_t on the left
I don't know what happened - I ran benchmarks locally before merging!
I'm sorry - no idea why it didn't reproduce on my machine. Maybe I messed something up
I can do the patch tommorow. It will take a while to merge it (judging by this merge experience).
Do you want to maybe roll this back?
Oct 29 2018
Oct 27 2018
I'm sorry, how do I merge this patch?
Updating tests according to review comments.
FYI, this patch fails to compile on my machine:
Oct 26 2018
Oct 25 2018
Oct 24 2018
Hi again! What is the situation with this PR?
Oct 16 2018
@EricWF can we maybe merge it now?
Oct 14 2018
__half_positve now just uses a cast to make_unsigned and other review issues.
Oct 13 2018
Seems like you are not comfortable with some additional changes that I did. I removed all of them and just left what was 100% necessary - casting to size_t for positive ptrdiff_t.
Simplifying the patch after review.
Oct 10 2018
Just wanted to know what is the current status of this PR.
Oct 1 2018
My bad, size_t cast is faster: http://quick-bench.com/klty7gya6LMs5p8YVNSQ3bnUXcA
Sep 30 2018
Minor comments typos.
- Addressed review comments.
- Changed (I measured - the result performs identically) cast to __half_unsigned function
- Added similar optimization and benchmarks to equal_range.
Sep 29 2018
Thanks for the quick review! I will address you comments tomorrow.
Unfortunately didn't manage to run a clang-format on my changes - it kept ignoring the library settings - so probably the formatting needs to be reviewed too.
Oct 30 2017
Oct 29 2017
I forgot the synopsis.