I managed to get ROOT v61803 compiling on Windows 64bit, and displaying the hsimple.C histogram.
Is this something the ROOT community is interested in integrating back into the master?
There are some potential discussions to be had on the handling of:
long natively being 4 bytes on Windows, where it is 8 bytes on *nixHi,
Thank you very much for your offer, I would be interested to see the patch. I already have a version compiling and running on Windows, but the diff is 731KB large and very intrusive. This is mostly due to changes of casting from cast to long to cast to intptr_t (or uintptr_t) a bit everywhere in the code, keeping it backward compatible and cross-platform…
And even if it compiles and run, there are still many tests failing. Did you try to enable testing and run CTest on your build? Could you share the results?
Cheers, Bertrand.
Hey Bertrand.
Here is my version of the Win64 port.
Most of the gtests are passing, except for the dataframe ones, which I could use help with. I suspect I am still missing some export symbols from libCling,
The ugly part is most of these fail with some sort of heap corruption.
Gtests on Win32
Total Test time (real) = 192.83 sec
The following tests FAILED:
8 - gtest-core-dictgen-test-dictgen-base (Failed)
14 - gtest-core-meta-test-testHashRecursiveRemove (Failed)
15 - gtest-core-meta-test-testTClass (Failed)
17 - gtest-core-metacling-test-coremetaclingtestUnit (Failed)
25 - gtest-hist-hist-test-testTKDE (Failed)
29 - gtest-io-io-test-TBufferMerger (Failed)
120 - gtest-tree-dataframe-test-dataframe-helpers (Failed)
136 - gtest-tree-tree-test-testBulkApi (Failed)
Win64
The following tests FAILED:
8 - gtest-core-dictgen-test-dictgen-base (Failed)
14 - gtest-core-meta-test-testHashRecursiveRemove (Failed)
15 - gtest-core-meta-test-testTClass (Failed)
17 - gtest-core-metacling-test-coremetaclingtestUnit (Failed)
25 - gtest-hist-hist-test-testTKDE (Failed)
29 - gtest-io-io-test-TBufferMerger (Failed)
108 - gtest-tree-dataframe-test-dataframe-friends (Failed)
109 - gtest-tree-dataframe-test-dataframe-colnames (Failed)
110 - gtest-tree-dataframe-test-dataframe-cache (Failed)
111 - gtest-tree-dataframe-test-dataframe-callbacks (Failed)
112 - gtest-tree-dataframe-test-dataframe-histomodels (Failed)
113 - gtest-tree-dataframe-test-dataframe-interface (Failed)
115 - gtest-tree-dataframe-test-dataframe-regression (Failed)
116 - gtest-tree-dataframe-test-dataframe-simple (Failed)
117 - gtest-tree-dataframe-test-dataframe-snapshot (Failed)
118 - gtest-tree-dataframe-test-dataframe-utils (Failed)
120 - gtest-tree-dataframe-test-dataframe-helpers (Failed)
121 - gtest-tree-dataframe-test-dataframe-ranges (Failed)
122 - gtest-tree-dataframe-test-dataframe-leaves (Failed)
123 - gtest-tree-dataframe-test-dataframe-vecops (Failed)
125 - gtest-tree-dataframe-test-dataframe-take (Failed)
127 - gtest-tree-dataframe-test-datasource-more (Failed)
128 - gtest-tree-dataframe-test-datasource-root (Failed)
129 - gtest-tree-dataframe-test-datasource-trivial (Failed)
130 - gtest-tree-dataframe-test-datasource-csv (Failed)
136 - gtest-tree-tree-test-testBulkApi (Failed)
137 - gtest-tree-tree-test-testBulkApiMultiple (Failed)
138 - gtest-tree-tree-test-testBulkApiVarLength (Failed)
139 - gtest-tree-tree-test-testBulkApiSillyStruct (Failed)
146 - gtest-tree-treeplayer-test-treetreeplayertestUnit (Failed)Hi,
Thanks for sharing your code. After a quick look, I have a few remarks:
This change:
#ifdef _WIN64
typedef intptr_t Long_t; //Signed long integer 8 bytes (long)
typedef uintptr_t ULong_t; //Unsigned long integer 8 bytes (unsigned long)
Is not a good solution, since it forces you to comment out all functions/methods using (U)Long64_t to prevent conflicts with (u)intptr_t
You cannot change the interface in a backward incompatible way. One must keep the API inchaged on other platforms.
For example, changing this signature:
Long_t TCling::Calc(const char *line, EErrorCode *error)
into this:
Long64_t TCling::Calc(const char *line, EErrorCode *error)
is backward incompatible (same for changing methods parameters)
How did you run the tests?
Can you try to enable testing (when configuring with CMake) and then try (in the build directory):
ctest -j[number of processors] -C [Debug/Release]
(for example, I use ctest -j8 -C Debug)
As a general comment, the code must still compile on all platforms. And the user’s code must still compile without having to modify it to adapt to any of the changes made for Win64…
As I said, I have a backward compatible and (more or less) working solution, but as you can see it is non trivial to change the code without changing the ROOT API
Cheers, Bertrand.
I initially ran the tests using ‘ctest -j8 -C Debug’ and modified the cmake files to allow for an easier debugging experience
(https://github.com/pkthong/root/commit/f1ce8c730c28b577ce2a49da88858fafdff8df5e)
In debugging my stack corruption issues, I think I have found a major stumbling block related to the way Cling+LLVM resolves symbols, specifically the non-uniquely defined symbols from msvrt.dll and ucrtbase.dll.
@bellenot , you might have an idea, since you made the following change, which reversed the order of symbol lookup
(https://github.com/pkthong/root/commit/c1e4fd941be13df6b106f99ad038e778b939c761?diff=unified#diff-9be88db82ded17a68af326157fc224edR591)
In Win64, the LLVM name mangler is returning a symbol lookup of ‘free’, compared to _free in Win32. It easily finds ‘free’ in msvcrt.dll, unfortunately, it should be using ucrtbase.dll’s version.
In Win32, since the mangler returns _free, ultimately, the symbol lookup happens in PlatformWin.cpp, where the ‘_’ is removed before looking up the symbol, and modules are processed in load order.(https://github.com/pkthong/root/commit/c1e4fd941be13df6b106f99ad038e778b939c761?diff=split#diff-9be88db82ded17a68af326157fc224edR595)
Does your version of the Win64 port have these issues?
If you have a git branch handy, I’d be interested in seeing how you worked around the issue.
I still have several issues with Win64, and I still have to create a branch for the Win64 port of ROOT… I’ll keep you in touch.
Just to keep you up to date on where I am.
I see the following issues, I could use some help with:
dynamic_cast is failing. See stressIterator-interpreted
TTree::Draw(<3d>) is passing an invalid ‘this’ pointer to the TPolyMarker3d::PaintH3()
§ Failing tests with commit(https://github.com/pkthong/root/commit/c1d378c6a8c9493b9f15a6413980b88491122972)
The following tests FAILED:
4 - gtest-core-dictgen-test-dictgen-base (Failed)
10 - gtest-core-meta-test-testHashRecursiveRemove (Failed)
11 - gtest-core-meta-test-testTClass (Failed)
13 - gtest-core-metacling-test-coremetaclingtestUnit (Failed)
21 - gtest-hist-hist-test-testTKDE (Failed)
25 - gtest-io-io-test-TBufferMerger (Failed)
77 - test-stressgeometry (Timeout)
78 - test-stressgeometry-interpreted (Timeout)
79 - test-stresslinear (Failed)
80 - test-stresslinear-interpreted (Failed)
81 - test-stressgraphics (Failed)
82 - test-stressgraphics-interpreted (Failed)
83 - test-stresshistogram (Timeout)
84 - test-stresshistogram-interpreted (Timeout)
88 - test-stressvector-interpreted (Failed)
89 - test-stressmathcore (Failed)
90 - test-stressmathcore-interpreted (Failed)
92 - test-stressfit-interpreted (Failed)
96 - test-stressiterators-interpreted (Failed)
97 - test-stressinterpreter (Failed)
101 - test-delaunay (Failed)
103 - test-periodic-build (Failed)
106 - gtest-tree-dataframe-test-dataframe-cache (Failed)
112 - gtest-tree-dataframe-test-dataframe-simple (Failed)
113 - gtest-tree-dataframe-test-dataframe-snapshot (Failed)
116 - gtest-tree-dataframe-test-dataframe-helpers (Failed)
142 - tutorial-benchmarks (Failed)
143 - tutorial-cont-TListAndSTL (Failed)
146 - tutorial-dataframe-df002_dataModel (Failed)
156 - tutorial-dataframe-df012_DefinesAndFiltersAsStrings (Failed)
158 - tutorial-dataframe-df014_CSVDataSource (Failed)
159 - tutorial-dataframe-df015_LazyDataSource (Timeout)
163 - tutorial-dataframe-df019_Cache (Failed)
164 - tutorial-dataframe-df020_helpers (Failed)
168 - tutorial-dataframe-df024_Display (Failed)
170 - tutorial-dataframe-df027_SQliteDependencyOverVersion (Failed)
171 - tutorial-dataframe-df028_SQliteIPLocation (Failed)
172 - tutorial-dataframe-df029_SQlitePlatformDistribution (Failed)
173 - tutorial-dataframe-df030_SQliteVersionsOfROOT (Failed)
177 - tutorial-fit-ConfidenceIntervals (Failed)
182 - tutorial-fit-exampleFit3D (Failed)
195 - tutorial-fit-graph2dfit (Failed)
198 - tutorial-fit-multidimfit (Failed)
203 - tutorial-foam-foam_demopers (Failed)
235 - tutorial-gl-parallelcoordtrans (Failed)
240 - tutorial-graphics-AtlasExample (Timeout)
314 - tutorial-graphs-timeSeriesFromCSV_TDF (Failed)
366 - tutorial-hist-th2polyEurope (Timeout)
368 - tutorial-hist-th2polyUSA (Timeout)
394 - tutorial-math-exampleFunctor (Failed)
405 - tutorial-math-mathcoreGenVector (Failed)
408 - tutorial-math-mathcoreVectorCollection (Failed)
419 - tutorial-mlp-mlpRegression (Failed)
420 - tutorial-multicore-mp001_fillHistos (Failed)
421 - tutorial-multicore-mp101_fillNtuples (Failed)
422 - tutorial-multicore-mp102_readNtuplesFillHistosAndFit (Failed)
423 - tutorial-multicore-mp103_processSelector (Failed)
424 - tutorial-multicore-mp104_processH1 (Failed)
425 - tutorial-multicore-mp105_processEntryList (Failed)
426 - tutorial-multicore-mp201_parallelHistoFill (Failed)
450 - tutorial-spectrum-spectrumpainter (Failed)
452 - tutorial-tasks (Failed)
464 - tutorial-tree-clonesA_Event (Failed)
465 - tutorial-tree-drawsparse (Failed)
466 - tutorial-tree-hsimpleProxyDriver (Failed)
467 - tutorial-tree-hsimpleReader (Failed)
470 - tutorial-tree-jets (Failed)
471 - tutorial-tree-ntuple1 (Failed)
472 - tutorial-tree-parallelcoord (Failed)
473 - tutorial-tree-parallelcoordtrans (Failed)
475 - tutorial-tree-run_h1analysis (Timeout)
477 - tutorial-tree-staff (Failed)
482 - tutorial-tree-tree2 (Failed)So with your changes, here is what I get:
96% tests passed, 22 tests failed out of 601
Label Time Summary:
longtest = 1102.28 sec*proc (15 tests)
tutorial = 3425.70 sec*proc (452 tests)
Total Test time (real) = 824.34 sec
The following tests FAILED:
14 - gtest-core-metacling-test-coremetaclingtestUnit (Failed)
23 - gtest-hist-hist-test-testTKDE (Failed)
29 - gtest-io-io-test-TBufferMerger (Failed)
89 - gtest-roofit-roofitcore-test-testWorkspace (Failed)
90 - gtest-roofit-roofitcore-test-testRooDataHist (Failed)
95 - test-check-invalidptr (Failed)
103 - test-stress (Failed)
113 - test-stresshistogram-interpreted (Failed)
123 - test-stressroostats-interpreted (Failed)
125 - test-stressfit-interpreted (Failed)
129 - test-stressiterators-interpreted (Failed)
136 - test-periodic-build (Failed)
139 - gtest-tree-tree-test-testBulkApi (Failed)
140 - gtest-tree-tree-test-testBulkApiMultiple (Failed)
141 - gtest-tree-tree-test-testBulkApiVarLength (Failed)
142 - gtest-tree-tree-test-testBulkApiSillyStruct (Failed)
162 - tutorial-fit-combinedFit (Failed)
163 - tutorial-fit-exampleFit3D (Failed)
185 - tutorial-fit-multidimfit (Failed)
372 - tutorial-legacy-cont-TListAndSTL (Failed)
393 - tutorial-math-exampleFunctor (Failed)
420 - tutorial-roofit-rf102_dataimport (Failed)
Errors while running CTest
Interesting. I take it you adopted some of my changes into your own win64 branch? Or did you run mine as-is?
I should clarify that I mostly wanted to do a quick and dirty prototype of the win64 port, with hopes that some of the changes will make it into your branch. At least I know the win32gdk will not be an issue for 64bit.
Once we can get a common branch, I’d be interested in poking at the remaining tests and seeing why they are still failing.
I only took this commit (more or less) on top of our changes, but I this still need to be understood… 
And as I said, our modifications are much more complex than what you did (but I still have to dig deeper into your code)
To see the affect of the mangling from this commit, put a breakpoint here
You will find that without the commit, the search for c runtime functions, like (::free) always resolves in this loop.
With the fix, the symbol resolves here
(note: the comment above on L611 is incorrect, and left over from an old commit)
I think I might have an answer for the dynamic_cast issue.
__ImageBase doesn’t seem to be handled correctly in the LLVM 5.0 version we have in ROOT.
This was fixed in the LLVM master in (https://reviews.llvm.org/D30709)
It may be worth a shot to upgrade LLVM to something newer, and retrying.
We were planning to upgrade LLVM since quite a while… Should happen soon(ish) Thanks for confirming it would solve at least one issue!
I imported ROOT 5.0.2 and applied the patches from (http://root.cern.ch/git/llvm.git) and backported the fix to __ImageBase, in case you wanted to try it out without waiting for the llvm upgrade.
I also fixed some ptr casting issues in mathcore.
(https://github.com/pkthong/root/commit/68da5daa0699e893f93eb38bb95b1f19bbc8d663)
Any movement on upgrading the LLVM stack in ROOT?
This should happen this year (sorry, but there no precise schedule for the upgrade)