poly1305/asm/poly1305-armv4.pl: avoid conditional branch to PLT in THUMB-2#5949
Closed
rahulchaudhry wants to merge 1 commit intoopenssl:masterfrom
rahulchaudhry:master
Closed
poly1305/asm/poly1305-armv4.pl: avoid conditional branch to PLT in THUMB-2#5949rahulchaudhry wants to merge 1 commit intoopenssl:masterfrom rahulchaudhry:master
rahulchaudhry wants to merge 1 commit intoopenssl:masterfrom
rahulchaudhry:master
Conversation
dot-asm
reviewed
Apr 13, 2018
Contributor
There was a problem hiding this comment.
I'd like to suggest (which is basically euphemism for insist:-) on removing this relocation altogether. I mean suggested change replaces one relocation type with another, but why have relocation at all if we can get rid of it? So drop #ifdef, beq .Lpoly1305_blocks and add .Lpoly1305_blocks: right after poly1305_blocks:.
…UMB-2. When compiling for THUMB-2, the conditional branch to PLT results in a R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation in THUMB-2 (ld.gold), while others can end up truncating the relocation to fit (ld.bfd). Convert this branch through PLT into a direct branch that the assembler can resolve locally. See android/ndk#337 for background. The current workaround is to disable poly1305 optimization assembly, which is not optimal and can be reverted after this patch: https://github.com/freedesktop/gstreamer-cerbero/commit/beab607d2b1ff23c41b7e01aa9c64be5e247d1e6 CLA: trivial
dot-asm
approved these changes
Apr 14, 2018
dot-asm
added
branch: master
Contributor
|
Editorial note. I'd say that "remove unintentional relocation" would be more appropriate subject. Because modification doesn't affect Thumb-2 exclusively and original subject is too "tech". I mean not everybody keeps those abbreviations in "Level 1 cache" of their brains. I would also modify description as "conditional branch to global symbol results in reference to PLT and a R_ARM_THM_JUMP19 relocation". You don't have to do anything, I can do it upon commit to repo. |
Author
|
Thanks |
Contributor
|
Ping for 2nd review. |
richsalz
approved these changes
Apr 18, 2018
richsalz
added
approval: done
levitte
pushed a commit
that referenced
this pull request
Apr 18, 2018
Branch to global symbol results in reference to PLT, and when compiling for THUMB-2 - in a R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation (ld.gold), while others can end up truncating the relocation to fit (ld.bfd). Convert this branch through PLT into a direct branch that the assembler can resolve locally. See android/ndk#337 for background. The current workaround is to disable poly1305 optimization assembly, which is not optimal and can be reverted after this patch: https://github.com/freedesktop/gstreamer-cerbero/commit/beab607d2b1ff23c41b7e01aa9c64be5e247d1e6 CLA: trivial Reviewed-by: Andy Polyakov <[email protected]> Reviewed-by: Rich Salz <[email protected]> (Merged from #5949)
levitte
pushed a commit
that referenced
this pull request
Apr 18, 2018
Branch to global symbol results in reference to PLT, and when compiling for THUMB-2 - in a R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation (ld.gold), while others can end up truncating the relocation to fit (ld.bfd). Convert this branch through PLT into a direct branch that the assembler can resolve locally. See android/ndk#337 for background. The current workaround is to disable poly1305 optimization assembly, which is not optimal and can be reverted after this patch: https://github.com/freedesktop/gstreamer-cerbero/commit/beab607d2b1ff23c41b7e01aa9c64be5e247d1e6 CLA: trivial Reviewed-by: Andy Polyakov <[email protected]> Reviewed-by: Rich Salz <[email protected]> (Merged from #5949) (cherry picked from commit 5bb1cd2)
Contributor
|
Merged. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When compiling for THUMB-2, the conditional branch to PLT results in a
R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation
in THUMB-2 (ld.gold), while others can end up truncating the relocation
to fit (ld.bfd).
Adding an "it eq" before the branch converts it into an unconditional
branch, which uses R_ARM_THM_JUMP24 relocation that has a range of 16MB.
See android/ndk#337 for background.
The current workaround is to disable poly1305 optimization assembly,
which is not optimal and can be reverted after this patch:
https://github.com/freedesktop/gstreamer-cerbero/commit/beab607d2b1ff23c41b7e01aa9c64be5e247d1e6
CLA: trivial