Page MenuHomePhabricator

llvm/cmake/config.guess: add support for riscv32 and riscv64
ClosedPublic

Authored by gokturk on Oct 11 2019, 4:55 PM.

Details

Summary

LLVM configuration fails with 'unable to guess system type' on riscv64. Add support for detecting riscv32 and riscv64 systems.

Diff Detail

Event Timeline

gokturk created this revision.Oct 11 2019, 4:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2019, 4:55 PM
lenary added a subscriber: lenary.Oct 17 2019, 9:24 AM

Isn't the correct fix for this to update config.guess with the authoritative one from GNU?

curl "http://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD" -o llvm/cmake/config.guess

Isn't the correct fix for this to update config.guess with the authoritative one from GNU?

curl "http://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD" -o llvm/cmake/config.guess

The current config.guess is GPL-2 or later. The upstream config.guess is GPL-3. I was under the impression that LLVM upstream wanted to maintain a GPL-2 license. So I wrote my own version.

The current config.guess is GPL-2 or later. The upstream config.guess is GPL-3. I was under the impression that LLVM upstream wanted to maintain a GPL-2 license. So I wrote my own version.

I also thought that might be a problem, but there are two more config.guess files in the monorepo, and one of them is GPL-3 (polly/lib/External/isl/config.guess), but maybe that's not considered core to LLVM enough to matter? I'm not sure to what extent a configure script is considered to affect the general LLVM license...

The current config.guess is GPL-2 or later. The upstream config.guess is GPL-3. I was under the impression that LLVM upstream wanted to maintain a GPL-2 license. So I wrote my own version.

I also thought that might be a problem, but there are two more config.guess files in the monorepo, and one of them is GPL-3 (polly/lib/External/isl/config.guess), but maybe that's not considered core to LLVM enough to matter? I'm not sure to what extent a configure script is considered to affect the general LLVM license...

I couldn't find anything in the comment messages regarding a license issue. I do see that Haiku support was added just like this (https://github.com/llvm/llvm-project/commit/9590c532b892743040b7a3ea8a8308098e1aff1e#diff-becde2e85ab9b9a6321925ea7aec6815), instead of simply having config.guess updated from its upstream. There ought to be a reason why nobody updated it since 2011. Regardless, this patch sidesteps the license question and should be safe to apply.

This revision is now accepted and ready to land.Feb 12 2020, 9:43 AM
This revision was automatically updated to reflect the committed changes.

The current config.guess is GPL-2 or later. The upstream config.guess is GPL-3. I was under the impression that LLVM upstream wanted to maintain a GPL-2 license. So I wrote my own version.

I also thought that might be a problem, but there are two more config.guess files in the monorepo, and one of them is GPL-3 (polly/lib/External/isl/config.guess), but maybe that's not considered core to LLVM enough to matter? I'm not sure to what extent a configure script is considered to affect the general LLVM license...

That files comes from creating a distribution of ISL, using whatever version of autoconf is on the system. It contains the following passage:

# As a special exception to the GNU General Public License, if you
# distribute this file as part of a program that contains a
# configuration script generated by Autoconf, you may include it under
# the same distribution terms that you use for the rest of that
# program.  This Exception is an additional permission under section 7
# of the GNU General Public License, version 3 ("GPLv3").

This file is not used since we re-created the build system using cmake. I could remove it if deemed necessary.

By contrast, I am more surprised that we slapped a Apache-2 license on ConvertUTF.cpp/.h and nobody seem to care: D66390.

luismarques added a comment.EditedFeb 14 2020, 3:08 PM

Thanks for the insight @Meinersbur .
(edit: removed comment about reverting this; that was meant for D69869)