[cling] Fix TLS in the cling JIT #1208

Teemperor · 2017-10-20T00:48:36Z

TLS is currently not suppored in the JIT. However, it's possible to
enable emulated TLS support in LLVM which means that we now support
TLS across all architectures. The performance downsides of this
should be neglectiable and can be easily worked around (by merging
TLS variables into a single one).

Patch created with a lot of help from Lang Hames and Pavel Labath!

phsft-bot · 2017-10-20T00:49:08Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

vgvassilev · 2017-10-20T09:00:37Z

@phsft-bot build with flags -Dclingtest=On

phsft-bot · 2017-10-20T09:01:06Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=On
How to customize builds

phsft-bot · 2017-10-20T10:09:15Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T10:12:33Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T10:28:54Z

Build failed on slc6/gcc49.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T10:39:31Z

Build failed on slc6/gcc62.
See console output.

Warnings:

googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1522:28: warning: the compiler can assume that the address of ‘val1’ will never be NULL [-Waddress]
googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1510:3: warning: nonnull argument ‘val1’ compared to NULL [-Wnonnull-compare]

Failing tests:

phsft-bot · 2017-10-20T10:52:10Z

Build failed on centos7/gcc49.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T10:58:19Z

Build failed on mac1012/native.
See console output.

Warnings:

/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]

And 10 more

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T11:11:06Z

Build failed on slc6/gcc62.
See console output.

Warnings:

googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1522:28: warning: the compiler can assume that the address of ‘val1’ will never be NULL [-Waddress]
googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1510:3: warning: nonnull argument ‘val1’ compared to NULL [-Wnonnull-compare]

Failing tests:

projectroot.interpreter.clingtest_check_cling

dpiparo · 2017-11-02T08:00:29Z

Hi, merging this will unblock the TFuture/Async building blocks for parallelism. Is there any blocker preventing us from proceeding with the merge?

phsft-bot · 2017-11-02T08:45:20Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=On
How to customize builds

vgvassilev · 2017-11-02T08:54:22Z

I'd like to put that in after PR #1218. I expect that would happen by the end of the week.

phsft-bot · 2017-11-02T09:24:30Z

Build failed on slc6/gcc62.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-11-02T09:40:29Z

Build failed on centos7/gcc49.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-11-02T09:48:53Z

Build failed on slc6/gcc49.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-11-02T09:49:41Z

Build failed on slc6/gcc62.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-11-02T10:21:09Z

Build failed on mac1012/native.
See console output.

Warnings:

/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]

And 10 more

Failing tests:

phsft-bot · 2017-11-02T11:27:13Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-11-02T11:33:49Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

vgvassilev · 2017-11-03T13:15:06Z

@phsft-bot build!

@Teemperor, could you fix formatting?

phsft-bot · 2017-11-03T13:15:35Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2017-11-03T14:25:27Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

vgvassilev · 2017-11-03T22:19:48Z

@phsft-bot build!

phsft-bot · 2017-11-03T22:20:17Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

Axel-Naumann · 2017-11-06T10:58:46Z

interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp

-                                        llvm::TargetOptions(),
-                                        Optional<Reloc::Model>(), CMModel,
-                                        OptLevel));
+  auto TM = std::unique_ptr<TargetMachine>(TheTarget->createTargetMachine(


Could you please split formatting and functional change into two commits? I'm terrible at filtering white-space parts of diffs from functional ones... Thanks, much appreciated!

Alright, done.

TLS is currently not suppored in the JIT on some platforms. However, it's possible to enable emulated TLS support in LLVM which means that we now support TLS across many architectures. The performance downsides of this are the overhead of accessing the variable due to the additional indirection by the emulation. However, this overhead is minimal and shouldn't affect most programs. It also can be easily worked around from the user side. This can be donefFor example by wrapping TLS variables into a single TLS struct variable that then contains the other variables. Or just minimizing referencing the TLS variable and use a normal copy of the variable instead and write it back later. Patch created with a lot of help from Lang Hames and Pavel Labath!

phsft-bot · 2017-11-06T15:21:50Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2017-11-06T15:22:32Z

Build failed on windows10/vc15.
See console output.

vgvassilev · 2017-11-06T18:58:22Z

@phsft-bot build only on windows10/vc15

phsft-bot · 2017-11-06T18:58:29Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

vgvassilev · 2017-11-06T18:58:58Z

@phsft-bot build just on windows10/vc15

phsft-bot · 2017-11-06T18:59:06Z

Starting build on windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

Teemperor assigned Axel-Naumann Oct 20, 2017

Teemperor requested a review from Axel-Naumann October 20, 2017 00:48

Teemperor force-pushed the FixTLS branch from 82943a1 to 7779ae9 Compare November 2, 2017 08:44

Teemperor force-pushed the FixTLS branch from 7779ae9 to ed21f70 Compare November 3, 2017 14:24

Axel-Naumann reviewed Nov 6, 2017

View reviewed changes

Teemperor added 2 commits November 6, 2017 16:19

Fix formatting of the TLS commit

b83f93d

Teemperor force-pushed the FixTLS branch from ed21f70 to b83f93d Compare November 6, 2017 15:21

vgvassilev merged commit ff0a2ef into root-project:master Nov 6, 2017

Teemperor deleted the FixTLS branch December 19, 2017 08:39

[cling] Fix TLS in the cling JIT #1208

[cling] Fix TLS in the cling JIT #1208

Uh oh!

Conversation

Teemperor commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Uh oh!

vgvassilev commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Warnings:

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Warnings:

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Warnings:

Failing tests:

Uh oh!

dpiparo commented Nov 2, 2017

Uh oh!

phsft-bot commented Nov 2, 2017

Uh oh!

vgvassilev commented Nov 2, 2017

Uh oh!

phsft-bot commented Nov 2, 2017

Failing tests:

Uh oh!

phsft-bot commented Nov 2, 2017

Failing tests:

Uh oh!

phsft-bot commented Nov 2, 2017

Failing tests:

Uh oh!

phsft-bot commented Nov 2, 2017

Failing tests:

Uh oh!

phsft-bot commented Nov 2, 2017

Warnings:

Failing tests:

Uh oh!

phsft-bot commented Nov 2, 2017

Failing tests:

Uh oh!

phsft-bot commented Nov 2, 2017

Failing tests:

Uh oh!

vgvassilev commented Nov 3, 2017

Uh oh!

phsft-bot commented Nov 3, 2017

Uh oh!

phsft-bot commented Nov 3, 2017

Uh oh!

vgvassilev commented Nov 3, 2017

Uh oh!

phsft-bot commented Nov 3, 2017

Uh oh!

Axel-Naumann Nov 6, 2017

Choose a reason for hiding this comment

Uh oh!

Teemperor Nov 6, 2017

Choose a reason for hiding this comment

Uh oh!