From 6d65456e302c386f0304fefd558554e7ff7a15c8 Mon Sep 17 00:00:00 2001 From: Andrew D Smith Date: Sat, 1 Jun 2024 18:14:09 -0700 Subject: [PATCH 1/2] Makefile.am: adding warnings and ignoring unknown attributes for the optimization __attribute__ in AbismalAlign.hpp --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 16979c5..f94a601 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,6 +38,7 @@ install installdirs: SUBDIRS := $(filter-out src/smithlab_cpp, $(SUBDIRS)) AM_CPPFLAGS = -I $(top_srcdir)/src/smithlab_cpp -I $(top_srcdir)/src/bamxx AM_CXXFLAGS = $(OPENMP_CXXFLAGS) +AM_CXXFLAGS += -Wall -Wextra -Wpedantic -Wno-unknown-attributes if ENABLE_SHORT AM_CXXFLAGS += -DENABLE_SHORT endif From a4810f0eed636079ce6e77af6f7bfb3f56595469 Mon Sep 17 00:00:00 2001 From: Andrew D Smith Date: Sat, 1 Jun 2024 18:15:03 -0700 Subject: [PATCH 2/2] AbismalAlign.hpp: Changing the way loop vectorization is turned off for the from_diag functions to now use __attribute__ instead of pragmas and hopefully fewer compilers will have a problem with this --- src/AbismalAlign.hpp | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/AbismalAlign.hpp b/src/AbismalAlign.hpp index 3467038..ef75014 100644 --- a/src/AbismalAlign.hpp +++ b/src/AbismalAlign.hpp @@ -206,21 +206,15 @@ get_best_score(const std::vector &table, const size_t n_cells, return *best_cell_itr; } -// ADS: it seems like with g++-13, on macos ventura on intel hardware -// the dynamic vectorized optimization of -O3 might be too aggressive -// and makes this function have strange behavior. Placing this pragma -// here helps, and below we restore it to the `-O3` default. Probably -// should move to attribute syntax soon. - -#ifdef __APPLE__ -#pragma GCC push_options -// ADS: below this won't make sense if the user wants no optimizations -// at all... -#pragma GCC optimize ("O2") -#endif +// ADS: it seems like with some versions of GCC, the optimization from +// `-O3`, which seems to include loop vectorizations, breaks the +// function below and the other overload named `from_diag` +// below. Using the __attribute__ helps with GCC, and it should be +// ignored if not supported. template +__attribute__((optimize("no-tree-loop-vectorize"))) void from_diag(T next_row, const T next_row_end, T cur_row, QueryConstItr query_seq, uint8_t ref_base) { @@ -251,6 +245,7 @@ from_left(T left_itr, T target, const T target_end) { /********* SAME FUNCTIONS AS ABOVE BUT WITH TRACEBACK ********/ template +__attribute__((optimize("no-tree-loop-vectorize"))) void from_diag(T next_row, const T next_row_end, T cur_row, QueryConstItr query_seq, uint8_t ref_base, U traceback) { @@ -289,9 +284,6 @@ from_left(T left_itr, T target, const T target_end, U traceback) { } } -#ifdef __APPLE__ -#pragma GCC pop_options -#endif inline void make_default_cigar(const uint32_t len, std::string &cigar) {