Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

Build hostrpc with fewer includes, nogpulib #171

Open
wants to merge 1 commit into
base: aomp11
Choose a base branch
from

Conversation

JonChesterfield
Copy link
Contributor

I don't believe the includes are required, and passing -nogpulib works around failures to find an appropriate rocm install.

@JonChesterfield
Copy link
Contributor Author

Now built on various systems. The file this affects is hostrpc-amdgcn.bc, which doesn't (and need not) use anything from rocm.

@ghost
Copy link

ghost commented Oct 14, 2020

DeepCode failed to analyze this pull request

Something went wrong despite trying multiple times, sorry about that.
Please comment this pull request with "Retry DeepCode" to manually retry, or contact us so that a human can look into the issue.

@JonChesterfield JonChesterfield changed the base branch from amd-stg-openmp to aomp11 October 14, 2020 19:29
@gregrodgers gregrodgers self-requested a review October 22, 2020 16:16
Copy link
Contributor

@gregrodgers gregrodgers left a comment

Choose a reason for hiding this comment

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

LGTM

@gregrodgers
Copy link
Contributor

JC, merge this if you are ready.

Copy link
Contributor

@estewart08 estewart08 left a comment

Choose a reason for hiding this comment

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

Lines 31-45 can be removed as well.

@@ -101,13 +101,10 @@ macro(add_openmp_library libname archname dir)
-fopenmp-targets=${triple}
-Xopenmp-target=${triple}
-march=${GPU}
-nogpulib
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we can add -nogpuinc here as well to avoid looking for hip runtime.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants