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:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  David Malcolm Originator Email:  -email is unavailable-
Open/Closed:  Closed Release:  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

jit_enable (1)
jit_startcnt (1)
x = [1:5];
for i = 1:5
   y(i) = exp (x(i));
endfor

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 Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dac922 (Posted a comment)
  • -email is unavailable- added by dmalcolm (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-02-12 rik5 StatusNone => Fixed
        Open/ClosedOpen => Closed

    Back to the top


    Powered by Savane 3.5