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

nvptx-none-as: Cope with ptxas not being available #18

Merged
merged 1 commit into from
May 4, 2017

Conversation

tschwinge
Copy link
Member

... to allow building GCC with PTX offloading even without CUDA being
installed.

Before, nvptx-none-as was calling the ptxas program to verify the assembly
is correct, which of course doesn't work very well when the proprietary
ptxas is not available. So the patch makes it invoke ptxas always only if
a new --verify option is used, if --no-verify is used, then as before it
is not invoked, and without either of these options the behavior is that if
ptxas is found in $PATH, then it invokes it, if not, it does only minimal
verification good enough for gcc/configure purposes (it turned out to be
sufficient to error out if .version directive is not the first non-comment
token (ptxas errors on that too).

... to allow building GCC with PTX offloading even without CUDA being
installed.

Before, nvptx-none-as was calling the ptxas program to verify the assembly
is correct, which of course doesn't work very well when the proprietary
ptxas is not available.  So the patch makes it invoke ptxas always only if
a new --verify option is used, if --no-verify is used, then as before it
is not invoked, and without either of these options the behavior is that if
ptxas is found in $PATH, then it invokes it, if not, it does only minimal
verification good enough for gcc/configure purposes (it turned out to be
sufficient to error out if .version directive is not the first non-comment
token (ptxas errors on that too).
Copy link
Member Author

@tschwinge tschwinge left a comment

Choose a reason for hiding this comment

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

@jakubjelinek, I'm not completely happy by the kind of nondeterminism this introduces (depending on whether ptxas happens to be found in PATH or not), but that's a minor point (as are my two review comments), so I'll merge this anyway. Sorry for the delay.

@@ -51,6 +51,7 @@ LIBS="$LIBS -lcuda"
AC_CHECK_FUNCS([[cuGetErrorName] [cuGetErrorString]])
AC_CHECK_DECLS([[cuGetErrorName], [cuGetErrorString]],
[], [], [[#include <cuda.h>]])
AC_CHECK_HEADERS(unistd.h sys/stat.h)
Copy link
Member Author

Choose a reason for hiding this comment

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

Minor point: there isn't actually a change related to unistd.h in this patch; no usage of HAVE_UNISTD_H.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, unistd.h is included unconditionally there (and used to be before too). Perhaps we could include it only conditionally.

@@ -897,9 +944,83 @@ fork_execute (const char *prog, char *const *argv)
do_wait (prog, pex);
}

/* Determine if progname is available in PATH. */
static bool
program_available (const char *progname)
Copy link
Member Author

Choose a reason for hiding this comment

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

In case that you copied this code (and the #defines further up in this file) from elsewhere, it would be good to note the source, to facilitate later maintenance.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is copied and modified from libiberty/make-relative-prefix.c. It is sufficiently different that sharing the code would require big changes into libiberty.

@tschwinge tschwinge merged commit f959110 into SourceryTools:master May 4, 2017
@tschwinge tschwinge deleted the jakub-no-ptxas branch May 4, 2017 17:05
configure.ac Show resolved Hide resolved
tschwinge added a commit that referenced this pull request Dec 21, 2020
]

This is a minor bug fix for commit 39511ba
"nvptx-none-as: Cope with ptxas not being available".

See
<39511ba#r546754640>:

| [...] we'll get: 'checking for ANSI C header files... no' if building
| nvptx-tools in a configuration where ['-lcuda'] is not available:
|
|     configure:3794: checking for ANSI C header files
|     configure:3814: gcc -c -g -O2   conftest.c >&5
|     configure:3814: $? = 0
|     configure:3887: gcc -o conftest -g -O2     conftest.c  -lcuda >&5
|     /usr/bin/ld: cannot find -lcuda
|     collect2: error: ld returned 1 exit status
|     configure:3887: $? = 1
|     configure: program exited with status 1
|     [...]
|     configure:3898: result: no
|
| ... or, in a configuration where building against the stub libcuda included in
| the CUDA toolkit ('--with-cuda-driver=[...]/cuda-6.5.14
| --with-cuda-driver-lib=[...]/cuda-6.5.14/lib64/stubs'), the execution test
| fails:
|
|     configure:3794: checking for ANSI C header files
|     configure:3814: gcc -c -g -O2 -I[...]/cuda-6.5.14/include  conftest.c >&5
|     configure:3814: $? = 0
|     configure:3887: gcc -o conftest -g -O2 -I[...]/cuda-6.5.14/include  -L[...]/cuda-6.5.14/lib64/stubs  conftest.c  -lcuda >&5
|     configure:3887: $? = 0
|     configure:3887: ./conftest
|     ./conftest: error while loading shared libraries: libcuda.so.1: cannot open shared object file: No such file or directory
|     configure:3887: $? = 127
|     configure: program exited with status 127
|     [...]
|     configure:3898: result: no

This doesn't seem to cause any actual problems, but it's still confusing to see
this 'configure'-time inconsistency between different build configurations.
tschwinge added a commit that referenced this pull request Jan 12, 2021
…n diagnostics [#18]

We're scanning the token stream of the input file after all.
tschwinge added a commit that referenced this pull request Jan 15, 2021
There is no reason not to, and it gives more consistent error reporting.
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.

None yet

2 participants