Skip to content

Conversation

@tcojean
Copy link
Contributor

@tcojean tcojean commented Aug 14, 2023

Adds a new option +sde which enables PAPI Software Define Events (SDE).

This allows other libraries to access software-defined Ginkgo counters through the standard PAPI interfaces.
Currently, only the most recent Ginkgo version (develop) and the most recent PAPI (master) is supported.

Comment on lines 75 to 76
conflicts("+sde", when="@:1.6.0")
conflicts("+sde", when="@master")
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I don't get it. Are you adding a variant that always conflicts?

Copy link
Member

@alalazo alalazo Nov 13, 2023

Choose a reason for hiding this comment

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

Oh, this works only on develop Can you make it clear and enable the variant only when=@develop and remove the conflicts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, this is not the case anymore after #40874 I will update it.

@alalazo alalazo self-assigned this Nov 13, 2023
Comment on lines 75 to 76
conflicts("+sde", when="@:1.6.0")
conflicts("+sde", when="@master")
Copy link
Member

@alalazo alalazo Nov 13, 2023

Choose a reason for hiding this comment

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

Oh, this works only on develop Can you make it clear and enable the variant only when=@develop and remove the conflicts?

variant("sycl", default=False, description="Enable SYCL backend")
variant("develtools", default=False, description="Compile with develtools enabled")
variant("hwloc", default=False, description="Enable HWLOC support")
variant("sde", default=False, description="Enable PAPI SDE support")
Copy link
Member

Choose a reason for hiding this comment

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

You can also:

Suggested change
variant("sde", default=False, description="Enable PAPI SDE support")
variant("sde", default=False, description="Enable PAPI SDE support", when="@1.7:")

Comment on lines 80 to 84
conflicts("+sde", when="@:1.6.0")

Copy link
Member

Choose a reason for hiding this comment

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

And delete these lines:

Suggested change
conflicts("+sde", when="@:1.6.0")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I forgot I could do it this way

alalazo
alalazo previously approved these changes Feb 22, 2024
@alalazo alalazo enabled auto-merge (squash) February 22, 2024 10:13
auto-merge was automatically disabled February 27, 2024 12:21

Head branch was pushed to by a user without write access

Signed-off-by: Terry Cojean <[email protected]>
Co-authored-by: Massimiliano Culpo <[email protected]>
@alalazo
Copy link
Member

alalazo commented Feb 28, 2024

@spackbot run pipeline

@spackbot-app
Copy link

spackbot-app bot commented Feb 28, 2024

I've started that pipeline for you!

@alalazo alalazo merged commit 86b4a86 into spack:develop Feb 29, 2024
mathomp4 pushed a commit to mathomp4/spack that referenced this pull request Mar 27, 2024
Signed-off-by: Terry Cojean <[email protected]>
Co-authored-by: Massimiliano Culpo <[email protected]>
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
Signed-off-by: Terry Cojean <[email protected]>
Co-authored-by: Massimiliano Culpo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants