[PATCH v1 0/1] libgccjit: Add a function to check if LTO is supported

[Sam James via Sourceware Forge]

Sam James (sjames) <sjames@sourceware.org>) commented on the code:


> +++ gcc/jit/libgccjit.cc
> @@ -4709,0 +4712,4 @@
> +{
> +#ifdef ENABLE_LTO
> +  char lto1_path[PATH_MAX];
> +  snprintf (lto1_path, sizeof (lto1_path), "%s%s/%s/lto1",
I'm a little bit surprised to see this -- why do you need to verify you can find `lto1`? Is there a scenario where you had GCC built with `--enable-lto` (=> `ENABLE_LTO`) but `lto1` was missing (maybe broken packaging)?

> +++ gcc/jit/libgccjit.cc
> @@ -4709,0 +4712,4 @@
> +{
> +#ifdef ENABLE_LTO
> +  char lto1_path[PATH_MAX];
> +  snprintf (lto1_path, sizeof (lto1_path), "%s%s/%s/lto1",
I might be wrong here, but I was under the impression that we can just distribute `libgccjit.so` without the libexec stuff and it would just work while that would not work for gcc.
I guess we could assume that if LTO was enabled, that the lto frontend would be present, but since the LTO frontend is enabled by default, that could be error-prone.
What are your thoughts on this?

> +++ gcc/jit/libgccjit.cc
> @@ -4709,0 +4712,4 @@
> +{
> +#ifdef ENABLE_LTO
> +  char lto1_path[PATH_MAX];
> +  snprintf (lto1_path, sizeof (lto1_path), "%s%s/%s/lto1",
I hadn't thought about that (i.e. people thinking it's a standalone library). I think the check is reasonable if you can imagine someone trying to do that. Just mention that as motivation in the commit message?

(Especially for the say, "CI case", where people might assume they can just grab the .so and not worry about anything else, dunno.)


--
https://forge.sourceware.org/gcc/gcc-TEST/pulls/103#issuecomment-2975