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

fix(tree): move declarations to top of function #563

Closed
wants to merge 0 commits into from

Conversation

aws-nslick
Copy link
Contributor

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

Stacked PRs:


fix(tree): move declarations to top of function

c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.
Signed-off-by: Nicholas Sielicki [email protected]

@aws-nslick aws-nslick requested a review from a team as a code owner September 4, 2024 16:44
@aws-nslick aws-nslick changed the base branch from aws-nslick/stack/9 to master September 4, 2024 19:07
@aws-nslick aws-nslick changed the base branch from master to aws-nslick/stack/9 September 4, 2024 19:08
@aws-nslick aws-nslick changed the base branch from aws-nslick/stack/9 to master September 4, 2024 19:16
@aws-nslick aws-nslick changed the base branch from master to aws-nslick/stack/7 September 4, 2024 19:17
@aws-nslick aws-nslick changed the base branch from aws-nslick/stack/7 to master September 4, 2024 20:25
@aws-nslick aws-nslick changed the title tree: move declarations to top of function fix(tree): move declarations to top of function Sep 4, 2024
@aws-nslick aws-nslick changed the base branch from master to aws-nslick/stack/7 September 4, 2024 20:25
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 22, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

stack-info: PR: aws#563, branch: aws-nslick/stack/10
Signed-off-by: Nicholas Sielicki <[email protected]>
bwbarrett
bwbarrett previously approved these changes Sep 24, 2024
Copy link
Contributor

@bwbarrett bwbarrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My old school C99 brain is happy now.

aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 25, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

stack-info: PR: aws#563, branch: aws-nslick/stack/10
Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 25, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

stack-info: PR: aws#563, branch: aws-nslick/stack/10
Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 26, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

stack-info: PR: aws#563, branch: aws-nslick/stack/10
Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 26, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

stack-info: PR: aws#563, branch: aws-nslick/stack/10
Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 26, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 27, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 27, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit that referenced this pull request Sep 27, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
@aws-nslick aws-nslick force-pushed the aws-nslick/stack/10 branch 2 times, most recently from 38afa11 to 7915503 Compare September 27, 2024 22:45
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 28, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
@aws-nslick
Copy link
Contributor Author

There was a bug on rebase:

diff --git a/src/nccl_ofi_net.c b/src/nccl_ofi_net.c
index 17e6ae2e..bcd27566 100644
--- a/src/nccl_ofi_net.c
+++ b/src/nccl_ofi_net.c
@@ -277,7 +277,7 @@ int nccl_net_ofi_create_plugin(nccl_net_ofi_plugin_t **plugin_p)
         * resources. This initialization happens once per process, and thus it
         * does not matter which device is used to create the endpoint.
         */
-       device = plugin->get_device(*plugin_p, 0);
+       device = plugin->get_device(plugin, 0);
 
        ret = device->get_ep(device, &base_ep);
        if (ret != 0) {

Prior to plugin_p being written to plugin a few lines down. Fixed.

@aws-nslick
Copy link
Contributor Author

bot:aws:retest

aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 28, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 30, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Sep 30, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit to aws-nslick/nccl-net-ofi that referenced this pull request Oct 1, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
aws-nslick added a commit that referenced this pull request Oct 1, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

Signed-off-by: Nicholas Sielicki <[email protected]>
@aws-nslick aws-nslick closed this Oct 1, 2024
@aws-nslick aws-nslick force-pushed the aws-nslick/stack/10 branch from 30efcbf to e27abab Compare October 1, 2024 19:55
@aws-nslick aws-nslick deleted the aws-nslick/stack/10 branch October 1, 2024 19:55
aws-ofiwg-bot pushed a commit to aws-ofiwg-bot/aws-ofi-nccl that referenced this pull request Oct 4, 2024
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

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.

3 participants