patchGNU Octave - Patches: patch #10344, Experimental stack VM for GNU...

 
 

patch #10344: Experimental stack VM for GNU Octave

Submitter:  Petter <petter>
Submitted:  Sun 14 May 2023 10:25:20 PM UTC
   
 
Category:  Core : new feature Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 20 Jun 2023 02:58:35 AM UTC, comment #7: 

As I mentioned in the discourse thread (https://octave.discourse.group/t/wip-stack-vm-for-octave/2884/92) I pushed a series of changes based on the patches posted here.  Also as noted in the commit message for the last changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/994b527e0ffe) when I first applied the VM changes I accidentally used the original patch posted here instead of the one posted to comment #3.  Changeset 994b527e0ffe updates Octave with the additional diffs present in the later patch file except for the object pool classes.  They don't appear to be required by the other changes and I think we should consider them separately.

John W. Eaton <jwe>
Group administrator
Mon 12 Jun 2023 06:46:02 AM UTC, comment #6: 

Sounds good to me! (Forgot to login)

Petter <petter>
Mon 12 Jun 2023 06:42:39 AM UTC, comment #5: 

Sounds good to me!

Anonymous
Thu 08 Jun 2023 07:31:02 PM UTC, comment #4: 

I've been looking through the latest patch.  It's an impressive amount of work.  Thanks for taking on this project!

I have some comments but most so far are about coding style or variable and function names.  I haven't really explored the changes deeply enough to be able to offer much feedback on the structure of the evaluator itself.

Maybe at this point it would be best to go ahead with applying the current patch and then I can work through any issues I have one by one?  If you agree, then I can do the merge fairly soon and we can continue with the work in the Octave source tree.

John W. Eaton <jwe>
Group administrator
Wed 07 Jun 2023 08:58:15 PM UTC, comment #3: 

I have updated the patch. ("octave_32196.patch")

Some problems are solved. E.g. warnings getting spammed in _run_test_suite_ as well as some more esoteric problems.

The build flag 'ENABLE_VM_EVALUATOR' doesn't do anything and the
user need to run '__enable_vm_eval__(1)' or use '__vm_compile' for the VM to run as discussed in the Discourse thread.


(file #54823)

Petter <petter>
Wed 24 May 2023 10:15:35 PM UTC, comment #2: 

comment #1:

> However, I see a lot of warnings while running the test suite that I don't remember seeing before. [...]
> Is handling Octave warnings affected by the VM?


It should not affect anything if it is working properly. I.e. very few functions 'know' that the VM is running or calling them.
'__run_test_suite__' sets 'quiet' on for warnings, so those warnings should not be printed.

I looked into that and it seems like the VM does not restore the stack frame's 'auto var' SAVED_WARNING_STATES when unwinding it-self.
One of the tests enables warnings with the "local" option, which turned on warnings for the rest of the test-suite, rather than
disabling them again when unwinding the frame.

Running some test manually it seems like the VM gives the same warnings as no VM.

I have fixed that and will clean up the patch and submit it here at some point.

>
> I didn't look into the changes in any detail. Just a rather "nitty" remark: We usually don't claim copyright for past years when new files are added.
>


Makes sense =)

Petter <petter>
Sun 21 May 2023 11:16:24 AM UTC, comment #1: 

Thank you for that patch.

I tried to build on Windows on top of a current head of the default branch (hg id 28ef2991fb5a):
It compiles fine and the built-in test suite (mostly) passes. 👍

However, I see a lot of warnings while running the test suite that I don't remember seeing before. Some of them are VM related (they mention not yet implemented features of the VM). But others seem to be unrelated afaict. E.g.:

  libinterp\parse-tree\pt-tm-const.cc-tst ........................warning: implicit conversion from numeric to char
warning: called from
    __test__ at line 2 column 1
    test    __run_test_suite__>run_test_dir at line 216 column 44
    __run_test_suite__>run_test_dir at line 180 column 40
    __run_test_suite__ at line 74 column 40
    D:/repo/Octave/octave-2/.build/../test/fntests.m at line 59 column 1

warning: implicit conversion from numeric to char
warning: called from
    __test__ at line 2 column 1
    test    __run_test_suite__>run_test_dir at line 216 column 44
    __run_test_suite__>run_test_dir at line 180 column 40
    __run_test_suite__ at line 74 column 40
    D:/repo/Octave/octave-2/.build/../test/fntests.m at line 59 column 1
[...]


Or:

  general\integral.m .............................................warning: quadcc: Error tolerance not met.  Estimated error: 6.0494e-07
 pass   34/34
  general\integral2.m ............................................warning: quadcc: Error tolerance not met.  Estimated error: inf
warning: quadcc: Error tolerance not met.  Estimated error: inf
warning: quadcc: Error tolerance not met.  Estimated error: inf
warning: quadcc: Error tolerance not met.  Estimated error: inf
 pass   50/50


The tests still pass though.

One actual test failure that seems to be new is also `warning` related:

>>>>> processing D:\repo\Octave\octave-2\test\struct.tst
***** error <invalid dot name structure assignment>
 c(4, 4, 4, 4).name  = "a";
 c(3, 3, 3, 3).value = 1;
 c([1, 3], 2, :, [3, 4]).value = 1;
!!!!! error failed.
Expected <invalid dot name structure assignment>, but got <invalid assignment to cs-list outside multiple assignment>


Is handling Octave warnings affected by the VM?

I didn't look into the changes in any detail. Just a rather "nitty" remark: We usually don't claim copyright for past years when new files are added.

Markus Mützel <mmuetzel>
Group administrator
Sun 14 May 2023 10:25:20 PM UTC, original submission:  

"The patch adds a bytecode stack-based VM for executing Octave user functions.

The build flag ENABLE_VM_EVALUATOR sets the VM as the default evaluator.

The user command '__enable_vm_eval__(bool)' disables or enables the VM
dynamically."

From discussion in https://octave.discourse.group/t/wip-stack-vm-for-octave/2884/55

This patch is self-contained and builds on latest in the default branch (f3d12359f0e4).

Petter <petter>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54823:  octave_32196.patch added by petter (725KiB - text/plain)
file #54741:  octave_32121.patch added by petter (660KiB - text/plain)

 

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 mmuetzel (Posted a comment)
  • -email is unavailable- added by petter (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 logged-in users can vote.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-06-07 petter Attached File- Added octave_32196.patch, #54823
    2023-05-14 petter Attached File- Added octave_32121.patch, #54741

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code