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

feat(build): disable semantic interposition #624

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Conversation

aws-nslick
Copy link
Contributor

@aws-nslick aws-nslick commented Sep 26, 2024

Stacked PRs:


feat(build): disable semantic interposition

Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki [email protected]

aws-nslick added a commit that referenced this pull request Sep 26, 2024
Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki <[email protected]>

stack-info: PR: #624, branch: aws-nslick/stack/37
@aws-nslick
Copy link
Contributor Author

Frustratingly, this PR needed to be reopened so that it could be rebased atop #594. The old PR was #601

@bwbarrett previously commented:

-fPIC is already automagically added where needed (for the shared library, not for the executable) by libtool. No need to add it here.

I've removed it.

Also two comments around using -Wl,--linker-flag options in CFLAGS.

-Wl,--gc-sections should be in LDFLAGS, not CFLAGS.

same as above -- don't put linker flags in CFLAGS.

I have removed them. In general, I don't agree with autotools that these flags should be avoided. There are certain flags that cannot be expressed when LIBS/LDFLAGS/CFLAGS are kept entirely separate. eg: -Wl,--whole-archive -lwhatever -Wl,--no-whole-archive has no equivalent.

Sorry for the trouble, thank you.

bwbarrett
bwbarrett previously approved these changes Sep 26, 2024
@bwbarrett
Copy link
Contributor

Yeah, the split of LIBS and LDFLAGS is not as big of a deal to me. But passing linker flags to every compilation step is weird, which is why I called those out. The only time I have strong feelings on LIBS vs. LDFLAGS is that in-tree libtool archives need to be in LIBS for all the dependency analysis to work correctly. Other than that, do what makes sense between LIBS and LDFLAGS.

aws-nslick added a commit that referenced this pull request Sep 26, 2024
Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki <[email protected]>

stack-info: PR: #624, branch: aws-nslick/stack/37
@aws-nslick aws-nslick requested a review from a team as a code owner September 26, 2024 20:46
@aws-nslick aws-nslick changed the base branch from aws-nslick/stack/35 to master September 26, 2024 20:46
@aws-nslick aws-nslick dismissed bwbarrett’s stale review September 26, 2024 20:46

The base branch was changed.

aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 26, 2024
Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki <[email protected]>

stack-info: PR: aws#624, branch: aws-nslick/stack/37
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 26, 2024
Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki <[email protected]>

stack-info: PR: aws#624, branch: aws-nslick/stack/37
Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki <[email protected]>

stack-info: PR: #624, branch: aws-nslick/stack/37
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 27, 2024
Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki <[email protected]>

stack-info: PR: aws#624, branch: aws-nslick/stack/37
@aws-nslick
Copy link
Contributor Author

@aws-nslick aws-nslick merged commit f385b9a into master Sep 27, 2024
30 of 31 checks passed
@aws-nslick aws-nslick deleted the aws-nslick/stack/37 branch September 28, 2024 00:27
aws-ofiwg-bot pushed a commit to aws-ofiwg-bot/aws-ofi-nccl that referenced this pull request Oct 4, 2024
Tell the compiler to treat symbols marked explicitly as
NCCL_OFI_EXPORT_SYMBOL as preemptible, but not others. This gives the
compiler the freedom to assume that no external LD_PRELOAD'd library can
inject itself as an implementation of any given function in the library,
which means the compiler has the freedom to optimize across function.

Signed-off-by: Nicholas Sielicki <[email protected]>
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

Successfully merging this pull request may close these issues.

2 participants