bugGNU Octave - Bugs: bug #41560, Typo in jit-typeinfo.cc

 
 

bug #41560: Typo in jit-typeinfo.cc

Submitted by:  None
Submitted on:  Wed 12 Feb 2014 01:20:42 AM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: None
Status: FixedAssigned to: None
Originator Name: David MalcolmOriginator Email: -unavailable-
Open/Closed: ClosedRelease: dev
Operating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

Thu 13 Feb 2014 04:57:04 PM UTC, comment #4:

I don't think it makes any difference, since compiling functions doesn't work at all. Just check by enable jit debugging with debug_jit (1)

There's a workaround patch in bug #41312 file functions_compile.patch

Stefan Mahr <dac922>
Wed 12 Feb 2014 05:40:44 PM UTC, comment #3:

I tested JIT before and after the change and it was definitely calling the exp function. I ran

The first two lines are boilerplate to engage the JIT compiler.

The value of y was equivalent to exp ([1:5]) before and after the typo fix. At any rate, this was surely the right thing to do.

Rik <rik5>
Project Administrator
Wed 12 Feb 2014 05:15:19 PM UTC, comment #2:

FWIW, I think the issue was more serious: the parameter was passed as the 2nd param to register_intrinsic, aka "iid":
http://hg.savannah.gnu.org/hgweb/octave/file/634d9989bf7b/libinterp/corefcn/jit-typeinfo.cc#l2047
where it gets converted to :
llvm::Intrinsic::ID id = static_cast<llvm::Intrinsic::ID> (iid);

and then used in this call:
llvm::Function *ifun = llvm::Intrinsic::getDeclaration (module, id, llvm_args);

Hence I believe that calls to "exp" were being erroneously jit-compiled to "cos" calls, thus silently giving the wrong answers.

That said, your fix in 634d9989bf7b looks good; thanks!

David Malcolm <dmalcolm>
Wed 12 Feb 2014 04:39:36 PM UTC, comment #1:

Luckily, that second parameter seems to be a size parameter and the return size of cos (double) is the same as exp (also double). Nevertheless, I corrected it in this changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/634d9989bf7b).

Rik <rik5>
Project Administrator
Wed 12 Feb 2014 01:20:42 AM UTC, original submission:

In http://hg.savannah.gnu.org/hgweb/octave/file/1363d909c577/libinterp/corefcn/jit-typeinfo.cc#l1848:

register_intrinsic ("exp", llvm::Intrinsic::cos, scalar, scalar);

Note the use of "cos" in the 2nd argument, which appears to be a copy-and-paste error from a similar registration a few lines above.

Anonymous

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 12 Feb 2014 04:39:36 PM UTCrik5StatusNone=>Fixed
      Open/ClosedOpen=>Closed

    Back to the top


    Powered by Savane 3.1-cleanup1