This is an archive of the discontinued LLVM Phabricator instance.

Differential D20825

Don't force Python2.7
AbandonedPublic

Authored by ldrumm on May 31 2016, 11:14 AM.

Details

Reviewers
chandlerc
Summary

llvm's CMakeLists has a Special case to reject python versions != 2.7 due to a dependency on that version by lldb that is no longer required.

224948 was introduced to workaround this limitation in lldb. However it appears this is no needed, as lldb builds with python >= 2.7 since @zturner made improvements to the lldb's cmake in 249466, supporting python3 in lldb. Additionally, python 3.5 is required for Visual Studio 2015 debug builds with python bindings, so the reasons for this limitation seem obsolete.

I'm not sure whether there are other repercussions from disabling this check, so I'd appreciate comments with those more familiar with build system

Diff Detail

Event Timeline

ldrumm updated this revision to Diff 59092.May 31 2016, 11:14 AM
ldrumm retitled this revision from to Don't force Python2.7.
ldrumm updated this object.
ldrumm set the repository for this revision to rL LLVM.
ldrumm added reviewers: zturner, chandlerc.
ldrumm added subscribers: llvm-commits, zturner.
zturner edited edge metadata.May 31 2016, 11:18 AM

Does lit run properly under Python 3.5?

There's also the awkward issue that LLDB specifies its python libraries independently of how LLVM does. I think it's safe to remove this check for that reason alone (LLVM's python interpreter should have no effect on LLDB's).

What combination of {OS, Python Version} have you tested this on?

chandlerc edited edge metadata.May 31 2016, 11:27 AM
In D20825#444646, @zturner wrote:

Does lit run properly under Python 3.5?

I don't know if others have fixed it recently, but for a long time I have had to keep lit pinned to Python2.7 for it to work.

zturner added a comment.May 31 2016, 11:32 AM

Yea, I think if we're going to make this change, we need to verify that lit works AND set up a buildbot that runs lit with some version of Python 3.

zturner resigned from this revision.Jun 2 2016, 8:38 PM
zturner removed a reviewer: zturner.
ldrumm added a comment.Jun 3 2016, 7:03 AM

Sorry for the delay, I wanted to run tests...

In D20825#444646, @zturner wrote:

Does lit run properly under Python 3.5?

A quick glance at the lit python sources indicate that they're python3 ready for the most-part with some suspicious use of old-style classes in places - nothing too to worry about in this case, as they don't use any of the old MRO behaviour and are not subclassed anywhere I can see, but again - not code I'm familiar with.

In any case, building with cmake ../llvm -G Ninja -DPYTHON_EXECUTABLE:FILEPATH=$(which python3); ninja check-llvm gives me a pass result on Linux platforms (on Win32 I get millions of errors for missing grep), so I think that if there are any code paths that are not py3-compatible, they are not being used by check-llvm:

Testing Time: 146.65s
Expected Passes    : 16594
Expected Failures  : 129
Unsupported Tests  : 186

Snip from ps aux | grep lit shows that python3 is actually being used for the test run

/usr/bin/python3 ~/llvm-upstream/llvm/utils/lit/lit.py -sv --param llvm_site_config=~/llvm-upstream/build/test/lit.site.cfg --param llvm_unit_site_config=~/llvm-upstream/build/test/Unit/lit.site.cfg~/llvm-upstream/build/test

There's also the awkward issue that LLDB specifies its python libraries independently of how LLVM does. I think it's safe to remove this check for that reason alone (LLVM's python interpreter should have no effect on LLDB's).

What combination of {OS, Python Version} have you tested this on?

Please see below. I've tried to test this on the big 3, but the Mac I used seems to have a very old linker that fails some of the LTO tests, and the windows builder fails with "unable to find grep errors. In both cases the test-suite itself is fine, only the tools it calls out to are missing or (on the Macintosh) outdated.

I don't know if this counts as a successful result though.

Linux (Ubuntu 16.04 LTS): CPython2.7.11:

cmake ../llvm -G Ninja -DPYTHON_EXECUTABLE:FILEPATH=$(which python2)
[...]
ninja check-llvm
[...]
[2046/2046] Running the LLVM regression tests
Testing Time: 177.57s
  Expected Passes    : 16594
  Expected Failures  : 129
  Unsupported Tests  : 186

Linux (Ubuntu 16.04 LTS): CPython3.5.1:

cmake ../llvm -G Ninja -DPYTHON_EXECUTABLE:FILEPATH=$(which python3)
ninja check-llvm
[127/127] Running the LLVM regression tests
Testing Time: 201.90s
  Expected Passes    : 16594
  Expected Failures  : 129
  Unsupported Tests  : 186

win32 (Windows 7 Pro 64bit): CPython3.5.1 32bitWoW:

Too many failures to include (missing grep)
snip of output:

call "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64
cmake ..\llvm -G Ninja -DPYTHON_EXECUTABLE:FILEPATH=C:\Python27-64\python.exe
[...]
ninja check-llvm
[...]
********************
Testing: 0 .. 10.. 20.. 30..
FAIL: LLVM :: CodeGen/PowerPC/store-load-fwd.ll (6621 of 16916)
******************** TEST 'LLVM :: CodeGen/PowerPC/store-load-fwd.ll' FAILED ********************
Script:
--
C:/Users/luke/llvm-upstream/build-py27-64/./bin\llc.EXE < C:\Users\luke\llvm-upstream\llvm\test\CodeGen\PowerPC\store-load-fw
d.ll -march=ppc32 | C:/Users/luke/llvm-upstream/build-py27-64/./bin\not.EXE grep lwz
--
Exit Code: 1

Command Output (stdout):
--
Command 0: "C:/Users/luke/llvm-upstream/build-py27-64/./bin\llc.EXE" "-march=ppc32"
Command 0 Result: 0
Command 0 Output:
        .text
        .file   "<stdin>"
        .globl  test
        .p2align        2
        .type   test,@function
test:                                   # @test
.Lfunc_begin0:
        .cfi_startproc
# BB#0:
        li 4, 1
        stw 4, 0(3)
        li 3, 1
        blr
.Lfunc_end0:
        .size   test, .Lfunc_end0-.Lfunc_begin0
        .cfi_endproc


        .section        ".note.GNU-stack","",@progbits


Command 0 Stderr:


Command 1: "C:/Users/luke/llvm-upstream/build-py27-64/./bin\not.EXE" "grep" "lwz"
Command 1 Result: 1
Command 1 Output:


Command 1 Stderr:
Error: Unable to find `grep' in PATH: no such file or directory
[...]

win32 (Windows 7 Pro 64bit): CPython2.7.11 64bit MSVC 2015:

Same results as above

Darwin Mac OS X; Apple Clang 500.2.76 (LLVM 3.3); Brew CPython3.5.1:

cmake ../llvm -G Ninja -DPYTHON_EXECUTABLE:FILEPATH=$(which python2)
# [...]
ninja check-llvm
# [...]
[1/1] Running the LLVM regression tests
FAIL: LLVM-Unit :: Support/SupportTests/TrailingObjects.TwoArg (1471 of 16920)
******************** TEST 'LLVM-Unit :: Support/SupportTests/TrailingObjects.TwoArg' FAILED ********************
Note: Google Test filter = TrailingObjects.TwoArg
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from TrailingObjects
[ RUN      ] TrailingObjects.TwoArg
~/llvm-git/llvm/unittests/Support/TrailingObjectsTest.cpp:125: Failure
Value of: llvm::alignOf<double>()
  Actual: 8
Expected: llvm::alignOf<Class2>()
Which is: 1
[  FAILED  ] TrailingObjects.TwoArg (0 ms)
[----------] 1 test from TrailingObjects (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] TrailingObjects.TwoArg

 1 FAILED TEST

********************
FAIL: LLVM :: tools/lto/hide-linkonce-odr.ll (16917 of 16920)
******************** TEST 'LLVM :: tools/lto/hide-linkonce-odr.ll' FAILED ********************
Script:
--
~/llvm-git/build-py35/./bin/llvm-as ~/llvm-git/llvm/test/tools/lto/hide-linkonce-odr.ll -o ~/llvm-git/build-py35/test/tools/lto/Output/hide-linkonce-odr.ll.tmp.o
/usr/bin/ld -lto_library ~/llvm-git/build-py35/./lib/libLTO.dylib -dylib -arch x86_64 -macosx_version_min 10.10.0 -lSystem -o ~/llvm-git/build-py35/test/tools/lto/Output/hide-linkonce-odr.ll.tmp.dylib ~/llvm-git/build-py35/test/tools/lto/Output/hide-linkonce-odr.ll.tmp.o -save-temps  -undefined dynamic_lookup -exported_symbol _c -exported_symbol _b  -exported_symbol _GlobLinkonce
~/llvm-git/build-py35/./bin/llvm-dis ~/llvm-git/build-py35/test/tools/lto/Output/hide-linkonce-odr.ll.tmp.dylib.lto.opt.bc -o - | ~/llvm-git/build-py35/./bin/FileCheck --check-prefix=IR ~/llvm-git/llvm/test/tools/lto/hide-linkonce-odr.ll
~/llvm-git/build-py35/./bin/llvm-nm ~/llvm-git/build-py35/test/tools/lto/Output/hide-linkonce-odr.ll.tmp.dylib | ~/llvm-git/build-py35/./bin/FileCheck --check-prefix=NM ~/llvm-git/llvm/test/tools/lto/hide-linkonce-odr.ll
--
Exit Code: 1

Command Output (stderr):
--
ld: could not parse object file ~/llvm-git/build-py35/test/tools/lto/Output/hide-linkonce-odr.ll.tmp.o: 'Global not a pointer type!', using libLTO version 'LLVM version 3.3svn, from Apple Clang 5.0 (build 500.2.76)' file '~/llvm-git/build-py35/test/tools/lto/Output/hide-linkonce-odr.ll.tmp.o' for architecture x86_64

--

********************
FAIL: LLVM :: tools/lto/opt-level.ll (16918 of 16920)
******************** TEST 'LLVM :: tools/lto/opt-level.ll' FAILED ********************
Script:
--
~/llvm-git/build-py35/./bin/llvm-as ~/llvm-git/llvm/test/tools/lto/opt-level.ll -o ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.o
/usr/bin/ld -lto_library ~/llvm-git/build-py35/./lib/libLTO.dylib -arch x86_64 -dylib -mllvm -O0 -o ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.dylib ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.o
~/llvm-git/build-py35/./bin/llvm-nm -no-llvm-bc ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.dylib | ~/llvm-git/build-py35/./bin/FileCheck --check-prefix=CHECK-O0 ~/llvm-git/llvm/test/tools/lto/opt-level.ll
/usr/bin/ld -lto_library ~/llvm-git/build-py35/./lib/libLTO.dylib -arch x86_64 -dylib -mllvm -O2 -o ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.dylib ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.o
~/llvm-git/build-py35/./bin/llvm-nm -no-llvm-bc ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.dylib | ~/llvm-git/build-py35/./bin/FileCheck --check-prefix=CHECK-O2 ~/llvm-git/llvm/test/tools/lto/opt-level.ll
--
Exit Code: 1

Command Output (stderr):
--
ld: warning: -macosx_version_min not specified, assuming 10.8
ld: could not parse object file ~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.o: 'Function not a pointer type!', using libLTO version 'LLVM version 3.3svn, from Apple Clang 5.0 (build 500.2.76)' file '~/llvm-git/build-py35/test/tools/lto/Output/opt-level.ll.tmp.o' for architecture x86_64

--

********************
FAIL: LLVM :: tools/lto/print-stats.ll (16919 of 16920)
******************** TEST 'LLVM :: tools/lto/print-stats.ll' FAILED ********************
Script:
--
~/llvm-git/build-py35/./bin/llvm-as ~/llvm-git/llvm/test/tools/lto/print-stats.ll -o ~/llvm-git/build-py35/test/tools/lto/Output/print-stats.ll.tmp.o
/usr/bin/ld -lto_library ~/llvm-git/build-py35/./lib/libLTO.dylib -arch x86_64 -dylib -mllvm -stats -o ~/llvm-git/build-py35/test/tools/lto/Output/print-stats.ll.tmp.dylib ~/llvm-git/build-py35/test/tools/lto/Output/print-stats.ll.tmp.o 2>&1 | ~/llvm-git/build-py35/./bin/FileCheck --check-prefix=STATS ~/llvm-git/llvm/test/tools/lto/print-stats.ll
/usr/bin/ld -lto_library ~/llvm-git/build-py35/./lib/libLTO.dylib -arch x86_64 -dylib -o ~/llvm-git/build-py35/test/tools/lto/Output/print-stats.ll.tmp.dylib ~/llvm-git/build-py35/test/tools/lto/Output/print-stats.ll.tmp.o 2>&1 | ~/llvm-git/build-py35/./bin/FileCheck --check-prefix=NO_STATS ~/llvm-git/llvm/test/tools/lto/print-stats.ll
--
Exit Code: 1

Command Output (stderr):
--
~/llvm-git/llvm/test/tools/lto/print-stats.ll:8:10: error: expected string not found in input
; STATS: Statistics Collected
         ^
<stdin>:1:1: note: scanning from here
ld: warning: -macosx_version_min not specified, assuming 10.8
^
<stdin>:1:27: note: possible intended match here
ld: warning: -macosx_version_min not specified, assuming 10.8
                          ^

--

********************
Testing Time: 773.60s
********************
Failing Tests (4):
    LLVM-Unit :: Support/SupportTests/TrailingObjects.TwoArg
    LLVM :: tools/lto/hide-linkonce-odr.ll
    LLVM :: tools/lto/opt-level.ll
    LLVM :: tools/lto/print-stats.ll

  Expected Passes    : 16591
  Expected Failures  : 135
  Unsupported Tests  : 190
  Unexpected Failures: 4
FAILED: cd ~/llvm-git/build-py35/test && /usr/local/bin/python3 ~/llvm-git/llvm/utils/lit/lit.py -sv --param llvm_site_config=~/llvm-git/build-py35/test/lit.site.cfg --param llvm_unit_site_config=~/llvm-git/build-py35/test/Unit/lit.site.cfg ~/llvm-git/build-py35/test
ninja: build stopped: subcommand failed.

Darwin Mac OS X; Apple Clang 500.2.76 (LLVM 3.3): Brew CPython2.7.11:

Same result as above
zturner added a comment.Jun 3 2016, 7:21 AM

I will leave it to Chandler to make the final call, but I can imagine a couple possible options, with an increasing amount of additional work for each one:

  1. Change can't go in unless all platforms pass 100% and there is a bot running with Python 3
  2. You can enable this behind a CMake flag, like -DEXPERIMENTAL_PY3_SUPPORT
  3. You can enable it without the flag, but print a warning at CMake gen time.
  4. Enable it with no warning.

1 seems too strict and 4 seems too lenient, but I'll see what others say.

ldrumm added a comment.Nov 18 2016, 8:22 AM

@chandlerc any more thoughts on this?

ldrumm abandoned this revision.Dec 19 2016, 10:37 AM

Revision Contents

PathSize
5 lines

Diff 59092

CMakeLists.txt

Show First 20 Lines • Show All 444 Lines • ▼ Show 20 Lines
# By default, we target the host, but this can be overridden at CMake # By default, we target the host, but this can be overridden at CMake
# invocation time. # invocation time.
set(LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_HOST_TRIPLE}" CACHE STRING set(LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_HOST_TRIPLE}" CACHE STRING
"Default target for which LLVM will generate code." ) "Default target for which LLVM will generate code." )
set(TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}") set(TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}")
include(HandleLLVMOptions) include(HandleLLVMOptions)
# Verify that we can find a Python 2 interpreter. Python 3 is unsupported. # Verify that we can find a Python interpreter.
# FIXME: We should support systems with only Python 3, but that requires work
# on LLDB.
set(Python_ADDITIONAL_VERSIONS 2.7)
include(FindPythonInterp) include(FindPythonInterp)
if( NOT PYTHONINTERP_FOUND ) if( NOT PYTHONINTERP_FOUND )
message(FATAL_ERROR message(FATAL_ERROR
"Unable to find Python interpreter, required for builds and testing. "Unable to find Python interpreter, required for builds and testing.
Please install Python or specify the PYTHON_EXECUTABLE CMake variable.") Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
endif() endif()
▲ Show 20 Lines • Show All 360 Lines • Show Last 20 Lines