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

 
 

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

Submitted by:  Julien Bect <jbect>
Submitted on:  Tue 25 Jul 2017 08:38:00 PM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Build Failure
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: 4.2.1Operating System: GNU/Linux

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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:

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.

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:

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:

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>
Project 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...

Julien Bect <jbect>

 

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

Attach File(s):
   
   
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
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by kupiqu (Posted a comment)
  • -unavailable- added by asr
  • -unavailable- added by godfrey (Posted a comment)
  • -unavailable- added by jbect (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 15 Nov 2017 03:59:54 PM UTCjweStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Sat 04 Nov 2017 09:38:59 PM UTCjbectAttached File-=>Added default-jit-LLVM-v1.tar.gz, #42330
    Sat 28 Oct 2017 07:07:22 AM UTCjbectAttached File-=>Added default-jit-LLVM38-fix-syntax-error.patch, #42280
    Fri 06 Oct 2017 02:23:31 AM UTCasrCarbon-Copy-=>Added asr
    Mon 31 Jul 2017 10:57:24 AM UTCjbectAttached File-=>Added stable-jit-LLVM38-patches-v5.tar.gz, #41372
    Wed 26 Jul 2017 08:23:04 PM UTCjbectAttached File-=>Added stable-jit-LLVM38-patches-v2.tar.gz, #41349
    Tue 25 Jul 2017 08:38:00 PM UTCjbectAttached File-=>Added stable-jit-LLVM38-patches.tar.gz, #41334
      Attached File-=>Added test_pisum.m, #41335

    Back to the top


    Powered by Savane 3.1-cleanup1