bugGNU Octave - Bugs: bug #51574, Build failure with --enable-jit...

 
 

bug #51574: Build failure with --enable-jit with LLVM 3.8

Submitter:  Julien Bect <jbect>
Submitted:  Tue 25 Jul 2017 08:38:00 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Build Failure
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 4.2.1 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 03 Jan 2019 09:18:42 AM UTC, comment #27: 

Patches that make it possible to build with LLVM 3.9 and 4.0 are now available on the patch tracker: https://savannah.gnu.org/patch/?9741.

Julien Bect <jbect>
Wed 15 Nov 2017 11:28:38 PM UTC, comment #26: 

John:  Definitely right. And, made it easy for me to try
llvm 4.0.1 (Fedora 26 and 27). See bug #52414.

Michael Godfrey <godfrey>
Group Member
Wed 15 Nov 2017 07:42:51 PM UTC, comment #25: 

The way I see it, it's no worse than before...

John W. Eaton <jwe>
Group administrator
Wed 15 Nov 2017 07:29:28 PM UTC, comment #24: 

Wow... I didn't think you would push all the patches at once, some of them were really not polished enough...

Julien Bect <jbect>
Wed 15 Nov 2017 03:59:54 PM UTC, comment #23: 

I'm able to build and run the tests if I apply all 18 patches.  I edited the commit message for the last change and pushed them.  Closing this report as the build failure is now fixed.

John W. Eaton <jwe>
Group administrator
Mon 13 Nov 2017 11:00:06 AM UTC, comment #22: 

Julien,

Sorry about repeating... And, I understand the difficulties
that you are having. The old implementation was "just a
start."

Anyhow, it is great that you are getting this done!

Thanks,
Michael

Michael Godfrey <godfrey>
Group Member
Mon 13 Nov 2017 10:49:36 AM UTC, comment #21: 

Hi Michael,

Yes, of course, you already told me that (see comment #15 below), and I intend to look into 3.9, then 4.0, then 5.0... when I have time...

But, FYI, my difficulties do not come only from changes in LLVM.  There have been changes in LLVM, yes, but in Octave also, and all of them contributed to break several things in Octave's JIT because nobody cared about adapting it...

For sure, a big change in LLVM occured between LLVM 3.5 and 3.6, when they finally dropped the "old JIT"... but it was called "old JIT" since LLVM 3.0, so it wasn't exactly a surprise ;-)

@++

Julien Bect <jbect>
Mon 13 Nov 2017 10:10:37 AM UTC, comment #20: 

Julien: You mentioned "I will look at LLVM 3.9 later..."
Could I just add that the current llvm on Fedora 26 is
llvm-4.0.1-1. Sorry about this and I hope that the llvm
folks are learning not to introduce so many incompatibilities
in each new release.

Michael Godfrey <godfrey>
Group Member
Mon 13 Nov 2017 09:51:30 AM UTC, comment #19: 

Hi John,

In this case you can look directly at the patch series hosted here:

https://github.com/jbect/revive-octave-jit

With these patches, you should be able to build with LLVM 3.8 and run make check (just tested on Debian stretch with LLVM 3.8).

The first 8 patches are ready to be reviewed and (I hope) pushed to default.  They make it possible to build again with LLVM 3.3 -- 3.5, I believe (but error handling is broken, which is why some unit tests have to be commented out).

The next 10 patches make it possible to build with LLVM 3.8 (probably also 3.7 and 3.6 with minor changes), but are in a much more unpolished state and seem to introduce some problems with complex numbers (which is why I had to comment out some more unit tests) that I haven't understood yet.

I will look at LLVM 3.9 later...

Julien Bect <jbect>
Thu 09 Nov 2017 11:56:27 PM UTC, comment #18: 

I started to look at this but I can't build with the patches applied because my systems only have llvm 3.7 or newer.

John W. Eaton <jwe>
Group administrator
Sun 05 Nov 2017 11:11:24 AM UTC, comment #17: 

@Julien

I understand. And, past experience with LLVM updates
was quite discouraging. They seemed to show no interest
in maintaining compatibility between updates. I hope
that they are better behaved now.

I would also say that there should be no problem with
your submitting patches for the current dev system
since there is nothing working now and your patches
would only do anything if llvm is turned on at configure
time. Just indicate that the current state is for
testing only.

And, you might ask Rik what he thinks and if he would
be willing to take quick look at the patches.

Michael Godfrey <godfrey>
Group Member
Sun 05 Nov 2017 06:11:00 AM UTC, comment #16: 

@Michael

Yes, it means NOT LLVM > 3.5.  In LLVM 36 and later the "old JIT" has been removed which means rewriting a lot of things.

I have already managed to compile and get encouraging results with LLVM 3.8 (and perhaps does it also work with LLVM 4.0 ?) but the patches are still a bit rough and currently they introduce a bug that I am trying to fix...

As I said, this is a first step, but it would already be nice if  one maintainer could review this and apply it to default (or tell me what I need to improve).

@++

Julien Bect <jbect>
Sun 05 Nov 2017 12:13:15 AM UTC, comment #15: 

Julien:

I assume that this means NOT with > 3.5
Fedora 26 is at 4.0.1

Of course, getting to to work at all again is a BIG help.

Thanks

Michael Godfrey <godfrey>
Group Member
Sat 04 Nov 2017 09:38:59 PM UTC, comment #14: 

As a first step, please consider the attached series of patches, to be applied on the default branch.

They make it possible to build the JIT again with LLVM 3.2 -- 3.5 (perhaps older, but I haven't tried).


(file #42330)

Julien Bect <jbect>
Wed 01 Nov 2017 06:35:22 PM UTC, comment #13: 

I see. More complicated loops (than the small attached example) can make the llvm compilation to fail, and then there is no JIT benefit...

Thank you All for clarifying this to me and especially to Julien for willing to improve JIT compilation in Octave!

avlas <kupiqu>
Wed 01 Nov 2017 06:22:31 PM UTC, comment #12: 

It is true that the JIT, as it existed before it even stopped to compile, was limited to speeding up a very limited number of loops.

Getting it to compile, and understanding how it works, is a necessary first step towards compiling more loops ;-)

Julien Bect <jbect>
Wed 01 Nov 2017 06:18:55 PM UTC, comment #11: 

Really, why is that? it seems to speed up the for loops in the attached example (test_pisum.m). A bit confused.

avlas <kupiqu>
Wed 01 Nov 2017 06:16:18 PM UTC, comment #10: 

avlas: The JIT compiler doesn't really do anything useful, so although getting it to build again is nice, it doesn't make the JIT compiler actually work.

John W. Eaton <jwe>
Group administrator
Wed 01 Nov 2017 06:10:12 PM UTC, comment #9: 

Sounds great to me! :)

Perhaps you can send an email to the maintainers mailing list, so they help you in pushing the patch?

avlas <kupiqu>
Wed 01 Nov 2017 05:49:56 PM UTC, comment #8: 

Well, one step at a time ;-)

LLVM 3.8 seems to be a reasonable target, since it is the version currently available in Debian stable (strecth), testing (buster) and even unstable (sid).

...

(and if any maintainer is reading this: can you please consider pushing the very simple patch attached to Comment #6 ?)

Julien Bect <jbect>
Wed 01 Nov 2017 05:25:36 PM UTC, comment #7: 

Hi Julien

Thanks for your work on this. Looking forward to seeing JIT back in Octave soon! :)


I'm sure you are aware that llvm 5.0 has been just released. Wondering whether it would make sense to fix Octave's JIT implementation with llvm 5.

Thanks and best

avlas <kupiqu>
Sat 28 Oct 2017 07:07:22 AM UTC, comment #6: 

Hi everyone,

I am now working to make the default branch compile with LLVM 3.8 (instead of the stable branch).

Still lot of work to do, but I have reached a state where I can get the same results as in Comment #5 but on the default branch.

Please find attached a first tiny patch that can be applied on default.

@++
Julien

(file #42280)

Julien Bect <jbect>
Mon 31 Jul 2017 10:57:25 AM UTC, comment #5: 

Here is a significant improvement over the previous series of patches.

The difference between the old LLVM JIT (used in Max Brister's code) and the newer MC-JIT is that a new module must be created each time.  It is not possible to modify a module after finalizeObject has been called.  This is explained here:

http://blog.llvm.org/2013/07/using-mcjit-with-kaleidoscope-tutorial.html

This new series of patches creates a new module for each jit_info/jit_function_info object, which solves the problem:


octave:1> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 11.7024 seconds.
octave:2> tic;  z = test_pisum2 (),  toc
z =  1.2021
Elapsed time is 15.2687 seconds.

octave:3> jit_enable (1)

octave:4> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 0.0859451 seconds.
octave:5> tic;  z = test_pisum2 (),  toc
z =  1.2021
Elapsed time is 0.0826061 seconds.
octave:6> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 0.088553 seconds.


BISTs still do not pass, but this is already much better ;-)


(file #41372)

Julien Bect <jbect>
Wed 26 Jul 2017 08:23:04 PM UTC, comment #4: 

Here is a slightly improved series of patch.

None of the real problems mentioned below are solved, but I'm starting to understand a little better what is goind on...

(file #41349)

Julien Bect <jbect>
Wed 26 Jul 2017 06:51:40 AM UTC, comment #3: 

I just found out that, in its present state, my patched version of stable is only able to jit one for loop, but not two of them...

Consider for instance the attached additional test function test_pisum2.m.  So now we have two very similar test functions, names test_pisum and test_pisum2.


octave:1> jit_enable (1)

octave:2> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 0.059464 seconds.

octave:3> tic;  z = test_pisum2 (),  toc
z =  1.2021
Elapsed time is 15.2592 seconds.

octave:4> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 0.024164 seconds.


The jit is able to optimize the first for loop, but ignores the second one.

And the results are reversed if I call test_pisum2 first:


octave:1&amp;gt; jit_enable (1)

octave:2&amp;gt; tic;  z = test_pisum2 (),  toc
z =  1.2021
Elapsed time is 0.052254 seconds.

octave:3&amp;gt; tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 11.835 seconds.

octave:4&amp;gt; tic;  z = test_pisum2 (),  toc
z =  1.2021
Elapsed time is 0.019443 seconds.


Obviously, there is something wrong... I have to understand better what is really going on.

Julien Bect <jbect>
Wed 26 Jul 2017 06:39:39 AM UTC, comment #2: 

Hello Michael,

Yes, I realize that it would be more natural to do this work on the default branch.

I have tried rebasing the pacthes there, but I get the following error:


In file included from libinterp/parse-tree/jit-typeinfo.h:36:0,
                 from libinterp/parse-tree/jit-ir.h:36,
                 from libinterp/template-inst/Array-jit.cc:34:
libinterp/parse-tree/jit-util.h:211:29: error: ‘OCT_MAKE_DECL_LIST’ has not been declared
   ret fname (JIT_PARAM_ARGS OCT_MAKE_DECL_LIST (type, arg, N)) isconst  \


Apparently OCT_MAKE_DECL_LIST has been removed, but I haven't yet tried to figure out what the proper replacement should be.

Could you (or anybody else) help me fix this issue?

@++
Julien

Julien Bect <jbect>
Tue 25 Jul 2017 09:24:07 PM UTC, comment #1: 

This looks very promising. One question: have you
tried applying your patches to the current development
system? This would be the natural place for wider testing.

Thanks very much for this contribution!

Michael

Michael Godfrey <godfrey>
Group Member
Tue 25 Jul 2017 08:38:00 PM UTC, original submission:  

Octave stable currently fails to build with LLVM 3.8 (and with all LLVM releases since LLVM 3.4 or 3.5, as far as I know).

I have managed to get it to build with the attached series of patches (currently in a very rough form).

make check currently crashes on jit.tst but at least I manage to get a significant speedup on the attached test function...


$ ./run-octave --no-gui
GNU Octave, version 4.2.1
[...]

octave:1> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 11.6463 seconds.

octave:2> jit_enable (1)

octave:3> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 0.054471 seconds.

octave:4> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 0.019253 seconds.

octave:5> tic;  z = test_pisum (),  toc
z =  1.6448
Elapsed time is 0.019371 seconds


Julien Bect <jbect>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #42330:  default-jit-LLVM-v1.tar.gz added by jbect (79KiB - application/gzip)
file #41334:  stable-jit-LLVM38-patches.tar.gz added by jbect (4KiB - application/gzip)
file #41335:  test_pisum.m added by jbect (137B - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by kupiqu (Posted a comment)
  • -email is unavailable- added by asr
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by jbect (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-11-15 jwe StatusNone Fixed
        Open/ClosedOpen Closed
    2017-11-04 jbect Attached File- Added default-jit-LLVM-v1.tar.gz, #42330
    2017-10-28 jbect Attached File- Added default-jit-LLVM38-fix-syntax-error.patch, #42280
    2017-10-06 asr Carbon-Copy- Added asr
    2017-07-31 jbect Attached File- Added stable-jit-LLVM38-patches-v5.tar.gz, #41372
    2017-07-26 jbect Attached File- Added stable-jit-LLVM38-patches-v2.tar.gz, #41349
    2017-07-25 jbect Attached File- Added stable-jit-LLVM38-patches.tar.gz, #41334
        Attached File- Added test_pisum.m, #41335

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code