bugGNU Octave - Bugs: bug #41312, latest Jit patch causes test jit...

 
 

bug #41312: latest Jit patch causes test jit failures

Submitted by:  Michael Godfrey <godfrey>
Submitted on:  Wed 22 Jan 2014 08:39:47 PM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Regression
Status: FixedAssigned to: None
Originator Name: Michael GodfreyOpen/Closed: Closed
Release: devOperating 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)

Mon 02 Feb 2015 04:24:13 PM UTC, comment #20:

I checked in the change to pt-jit.cc (http://hg.savannah.gnu.org/hgweb/octave/rev/885d38747cd5). I made the last two remaining failing tests in jit.tst %!xtests because support for complex function prototypes does not appear to exist yet. Closing report.

Rik <rik5>
Project Administrator
Tue 05 Aug 2014 03:53:33 PM UTC, comment #19:

This report has been idle for a while. Should functions_compile.patch be applied? It does decrease the error count by 2 when running 'make check', but there is an indication that perhaps this is not the correct fix.

I'm willing to commit whatever patch is agreed on.

Rik <rik5>
Project Administrator
Mon 27 Jan 2014 04:56:31 PM UTC, comment #18:

Would it not be better to fix the
behavior of clear ?

Another choice is to just delete the
offending clear. Doing that allows
all the tests to run correctly.

Was there a reason for the clear in the first place?

Michael Godfrey <godfrey>
Project Member
Mon 27 Jan 2014 03:49:01 PM UTC, comment #17:

I found the regression was introduced with rev 3f43e9d6d86e

Attached two patches. One fixes the regression above, but I'm not sure if it's done the right way.

The second patch fixes two cases when calling functions that have undefined variables.

The remaining two failing tests should be xfail/unimplemented, since I found no code for complex matrix support. There's no conditional xtest in test.m, so let the tests fail for now.

(file #30388, file #30389)

Stefan Mahr <dac922>
Thu 23 Jan 2014 04:09:09 PM UTC, comment #16:

Ah yes, I forgot the empty line significance. Thanks.
I am not too worried about the exact interpretation of
"regression." Just that the behavior is not worse than
what it was in, say, 3.8.0 which is the first time "users"
would have had a chance to try JIT. Thus, these failures
should not be viewed as blocking future releases as
long as JIT is not advertised as "ready for general use."

And, make check is running as it should.

Thanks.

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 03:30:51 PM UTC, comment #15:

The test script works slightly different. The tests are interpreted as blocks, every line has to start with %!. Empty lines break this block.

You can prove yourself by creating a file foo.tst

Well, actually I think the failing tests are regressions, but introduced long time ago. Since the old tests doesn't catch these errors, nobody ever noticed this.

Stefan Mahr <dac922>
Thu 23 Jan 2014 03:30:26 PM UTC, comment #14:

Actually, there are 6 cases where the computed result is
correct, but the jit_failure_count was raised. These
six need attention.

Very useful to have these additional tests!

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 03:18:37 PM UTC, comment #13:

Are you sure about that? I got a message about
"nested function definition not allowed..." until
I fixed that. But, I agree that the important
problem is the test that starts with:
m = [1 2 3]

Anyhow, it is good that essentially all the tests
pass. So, I do not think there is any regression.

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 03:02:47 PM UTC, comment #12:

Adding endfunction would be just a cosmetic change, it's not needed for the test.

Stefan Mahr <dac922>
Thu 23 Jan 2014 02:36:31 PM UTC, comment #11:

After commenting out the line: assert (jit_failure_count, 0);
at line 428 the script runs to line 565: shared id
where I get:
'shared' undefined near line 522 column 1

This is probably a consequence of running outside the
make check context.

But this is at the end of the tests.

So, it appears that the only failing test is:
%!testif HAVE_LLVM
%! jit_failure_count (0)
%! m = [1 2 3];
%! for i=1:1001
%! m = sin (m);
%! break;
%! endfor
%! assert (m == sin ([1 2 3]));
%! assert (jit_failure_count, 0);

which gets the right answer but raises the jit_failure_count.
The missing endfunction that I mentioned does need fixing.

Does this help?

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 02:30:16 PM UTC, comment #10:

This is intentional. octaves JIT code has overloads for JIT compiling the sin function, but it doesn't compile it. That's why it's necessary to show an error here.

Of course assert (m == sin ([1 2 3])); is correct, since if JIT compiling fails, there's a fall back to the traditional interpreter.

Stefan Mahr <dac922>
Thu 23 Jan 2014 02:19:26 PM UTC, comment #9:

I know a bit more:
If I run the full set of tests I do get a failure at
the test that starts at line 420 of jit.tst

If I run this function with debug_jit on I get:

octave:12> m = [1 2 3];
octave:13> for i=1:1001

> m = sin (m);
> break;
> endfor

jit fail: Missing overload in type inference for #9 = call (any: sin#8)
octave:14> assert (m == sin ([1 2 3]));
octave:15> assert (jit_failure_count, 0);
error: ASSERT errors for: assert (jit_failure_count,0)

Location | Observed | Expected | Reason
() 1 0 Abs err 1 exceeds tol 0
====================
So, this test gets the correct answer, but raises
jit_failure_count

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 01:55:02 PM UTC, comment #8:

I get:
octave:5> jit_enable (1); debug_jit (1); jit_failure_count (0);
octave:6> do a=sin(1); until(1)
jit fail: Missing overload in type inference for #2 = call (any: sin#19)
octave:7> jit_failure_count
ans = 1

I should clarify: My previous test was on only the first
section of jit.tst. I now find that there are failures later
on in the full jit.tst. At least part of the problems however
seem to be editing. Line 471 of jit.tst reads:
%!function test_compute_idom ()
but this function is not closed by an endfunction before
the next function declaration at line 492.

So, this is a bit more complicated than I had indicated.

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 01:19:08 PM UTC, comment #7:

Could you post the results of the following commands?

Stefan Mahr <dac922>
Thu 23 Jan 2014 12:52:04 PM UTC, comment #6:

I just edited jit.tst to remove the %!, and
jit was enabled. In this case I am sure that
there was no exception raised since as you said,
and the code says, that the jit_failure_count is
incremented. At the end of the tests the jit_failure_count
was still 0.

I turned on debug_jit(1) and the tests still run
with no debug output.

I also tried setting jit_startcnt =1. This had no effect.

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 11:04:37 AM UTC, comment #5:

How did you run the tests separately? Are you sure you enabled JIT? Please also enable debug_jit. If you see it throws an exception, than jit_failure_count is increased.

Stefan Mahr <dac922>
Thu 23 Jan 2014 04:53:15 AM UTC, comment #4:

If I go to octave/test
and run the tests separately they run and
jit_failure_count is left at 0.

If I try run jit.tst it also runs
without errors.

So, something goes wrong in the
make check handling.

Pretty sure that the tests run OK.

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 04:21:24 AM UTC, comment #3:

The failures are all due to
assert (jit_failure_count, 0);
failing because something has set jit_failure_count =1

The tests themselves (that I checked) return correct answers.
But, it is not clear that some of the tests
actually test jit operations, i.e. test 2 seems to
only loop 2 times!!

Good to look at jit_failure_count() wherever it is
defined. And, check the tests.

Michael Godfrey <godfrey>
Project Member
Thu 23 Jan 2014 01:30:19 AM UTC, comment #2:

Oh, very good.

I will look at thefailing cases more closely.

Thanks,
Michael

Michael Godfrey <godfrey>
Project Member
Wed 22 Jan 2014 10:21:48 PM UTC, comment #1:

I just improved the test code to see if there are some errors. The JIT code was broken before.

Stefan Mahr <dac922>
Wed 22 Jan 2014 08:39:47 PM UTC, original submission:

Attached is the jit failure section of fntests.log

Michael Godfrey <godfrey>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #30388:  functions_compile.patch added by dac922 (856B - text/x-patch)
file #30389:  jit_tst.patch added by dac922 (940B - text/x-patch)
file #30349:  jit_fails_22_Jan added by godfrey (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by dmalcolm
  • -unavailable- added by dac922 (Posted a comment)
  • -unavailable- added by godfrey (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 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 02 Feb 2015 04:24:13 PM UTCrik5StatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Mon 17 Mar 2014 08:30:03 PM UTCdmalcolmCarbon-Copy-=>Added dmalcolm
    Mon 27 Jan 2014 03:49:01 PM UTCdac922Attached File-=>Added functions_compile.patch, #30388
      Attached File-=>Added jit_tst.patch, #30389
    Wed 22 Jan 2014 08:39:47 PM UTCgodfreyAttached File-=>Added jit_fails_22_Jan, #30349

    Back to the top


    Powered by Savane 3.1-cleanup1