[FIX] gcc-15: Better bogus memcpy fix#3278
Conversation
|
Documentation preview available at https://docs.seqan.de/preview/seqan/seqan3/3278 |
| { | ||
| this->number_of_columns = number_of_columns.get(); | ||
| optimal_column.clear(); | ||
| horizontal_column.clear(); |
There was a problem hiding this comment.
Mhmm, I really don't see how this code should be problematic. These are simply two std::vectors, nothing special.
Can you tell in which context this diagnostic error occurs, e.g. the test cases for which it does not compile?
Right now, I would keep the GCC workaround thing and maybe move it up here instead. This clearly indicates that something is weird with the compilers and that we don't know what the actual cause of this error is. Yet, a std::vector::clear should not be a problem at all.
There was a problem hiding this comment.
https://cdash.seqan.de/build/282625
It's the score_matrix_single_column_simd_test
I will also try moving the macro here.
It's bogus, it should be fine with the clear. But there is this alternative to not use the macro but change the code.
There was a problem hiding this comment.
Moving the workaround to the resize also fixes both warnings
There was a problem hiding this comment.
I see. What I can tell from the compiler error is that the test is not built with SIMD instrutions enabled. Thus, there is this vector with aligned allocator defined as aligned_allocator<__vector(1) int, 4>. The last constant represents the byte alignment, which may cause some trouble here. Within the context of the SIMD alignment we would expect the type to have at least 16 byte alignment which would be the minimum for 128-bit SIMD operations.
Maybe the whole thing can be fixed altogether if we choose for the alignment in the aligned_allocator definition something like: std::max(alignof(value_t), alignof(std::max_align_t)).
At least for x86 architectures this would be at least 16 byte alignment.
There was a problem hiding this comment.
This would be changed to something like:
static constexpr size_t align_of_score_v = std::max(alignof(score_t), alignof(std::max_align_t));
using physical_column_t = std::vector<score_t, aligned_allocator<score_t, align_of_score_v>>;
There was a problem hiding this comment.
I am curious whether this fixes it as well.
There was a problem hiding this comment.
Ohh, yes that makes sense. Explains the 1 VS 4 bytes. I'll try compiling with -march=native and no workaround to see whether it works. Then I'll try your typedef.
Just as a side note, the nightly server is powerpc64le, so they have altivec instead of sse/avx.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3278 +/- ##
=======================================
Coverage 98.12% 98.12%
=======================================
Files 270 270
Lines 11926 11926
Branches 105 105
=======================================
Hits 11702 11702
Misses 224 224 ☔ View full report in Codecov by Sentry. |
rrahn
left a comment
There was a problem hiding this comment.
Looks good! Please merge at your will.
This partially reverts 33f3ebaIt fixes a new bogus memcpy error with gcc-15 (fedora build).But this one had a better stacktrace and also happens to fix the one from the linked commit.
From what I can tell, the compiler is confused by theoptimal_column.clear()(but nothorizontal_column.clear()). Seems like, for the diagnostic, it forgets that it was cleared and then weird stuff happens inresize()- only affects diagnostics, not the actual compiled code.