Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

smithWatermanBackTrack is being shared between AVX2 and AVX-512 #169

Open
philipc opened this issue Dec 27, 2021 · 2 comments
Open

smithWatermanBackTrack is being shared between AVX2 and AVX-512 #169

philipc opened this issue Dec 27, 2021 · 2 comments

Comments

@philipc
Copy link

philipc commented Dec 27, 2021

When compiling with gcc 11.2.0, smithWatermanBackTrack isn't actually inlined, and this results in both the AVX2 and AVX-512 implementations calling the same copy of it. This is bad because each copy needs to be specialized to use either AVX2 instructions or AVX-512 instructions. Here's the relevant bits from objdump -d build/native/libgkl_smithwaterman.so:

0000000000001610 <_Z19runSWOnePairBT_avx2iiiiPhS_ssaPciPjPi>:
    193c:       e8 8f f7 ff ff          call   10d0 <_Z22smithWatermanBackTrackP10dnaSeqPairiiiiPii@plt>

0000000000002a80 <_Z21runSWOnePairBT_avx512iiiiPhS_ssaPciPjPi>:
    2dec:       e8 df e2 ff ff          call   10d0 <_Z22smithWatermanBackTrackP10dnaSeqPairiiiiPii@plt>

I think it should at least be changed to static inline, and possibly renamed to CONCAT(smithWatermanBackTrack_,SIMD_ENGINE).

@Kmannth
Copy link
Contributor

Kmannth commented Jan 25, 2022

@philipc Thanks for this note. We build on gcc version 7.3.1 we see distinct functions for AVX2 and AVX512 as expected. Perhaps this is related to the GCC 11.2.0 stack.

From a gcc 7.3.1 I see today:

0000000000000da0 <_Z19runSWOnePairBT_avx2iiiiPhS_ssaPciPjPi>:
e54: 0f 86 ee 10 00 00 jbe 1f48 <_Z19runSWOnePairBT_avx2iiiiPhS_ssaPciPjPi+0x11a8>
...
0000000000002070 <_Z21runSWOnePairBT_avx512iiiiPhS_ssaPciPjPi>:
2124: 0f 86 5e 11 00 00 jbe 3288 <_Z21runSWOnePairBT_avx512iiiiPhS_ssaPciPjPi+0x1218>

@Kmannth
Copy link
Contributor

Kmannth commented Jan 25, 2022

Feel free to submit a patch.

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

No branches or pull requests

2 participants