> On 01/04/2019 02:35 PM, Rik wrote:

>> All,

>>

>> It's been a humbling experience going through the issues detected by static

>> analysis of the Octave code base

>> (

https://wiki.octave.org/PVS_static_analyzer_-_5.0_Release). Frankly, I

>> thought the code base was better than it turned out to be. Besides flat

>> out calling the wrong function (e.g., double_value instead of

>> xdouble_value), I just found and corrected a surprisingly simple but

>> pervasive performance hit for any exceptional value (-Inf, Inf, NaN).

>>

>> The code in question is deep in liboctave at liboctave/util/lo-ieee.cc.

>> The representation of an IEEE exceptional value on the particular HW Octave

>> is running on is found and then stored away so that subsequent uses just

>> retrieve that value rather than re-deriving it each time. The

>> initialization code was

>>

>> void

>> octave_ieee_init (void)

>> {

>> bool initialized = false;

>>

>> if (! initialized)

>> {

>> ... code to find and store exceptional values ...

>> initialized = true;

>> }

>> }

>>

>> As you can see, initialized is always false so the longer code to derive

>> the exceptional values is run every single time. And the exceptional

>> values themselves are functions which end up calling octave_ieee_init

>> twice: once for the "double" value and once for the "single" value.

>

> The compiler should have issued a "test always false" type of warning for

> that...

>

> There might be a small non-bug change in order beyond what you modified

> in Rev #26425. If I understand correctly, before the change something like

>

> x = Inf(1000);

>

> would have a performance hit because of the reinitialization for every

> instance of IEEE inf.

>

> However, there are multiple locations where there is assurance of

> initialization:

>

> libinterp/corefcn/interpreter.cc: octave_ieee_init ();

> liboctave/util/lo-ieee.cc: octave_ieee_init ();

> liboctave/util/lo-ieee.cc: octave_ieee_init ();

> liboctave/util/lo-ieee.cc: octave_ieee_init ();

> liboctave/util/lo-ieee.cc: octave_ieee_init ();

> liboctave/util/lo-ieee.cc: octave_ieee_init ();

> liboctave/util/lo-ieee.cc: octave_ieee_init ();

>

> The first check is when an instance of the interpreter is created, in the

> constructor. The remaining are when there is a particular call to

> actually set +/-Inf or NA. I would think that only one of those two

> groups is needed, but not both.

>

> If the IEEE initialization is checked only once upon creating the

> interpreter then Octave would avoid quite a bit of checking in the

> x=inf(1000) case, but that would also mean that Octave would immediately

> abort if the Inf/NA aren't present when an interpreter is created for any

> particular non-Inf/NA script.

>

> Instead, if the IEEE initialization is checked only whenever an Inf/NA

> assignment is done, then Octave would have a chance to run properly so

> long as it didn't need Inf/NA.

>

>

>> Also, despite their name, Octave uses exceptional values quite a bit. For

>> example, graphics handles are often initialized to NaN until they can be

>> assigned a true handle.

>

> Then that makes more of a case for having the octave_ieee_init() as

> mandatory in interpreter.cc. [One could temporarily force the

> occurrences of octave_ieee_init() in lo-ieee.cc to abort and see if that

> function is used anywhere outside the interpreter prior to instantiation

> of the interpreter.]

>

> Something else I'm wondering about is what would happen in the case where

> there is no octave_ieee_init() in interpreter::interpreter() and the

> NA/Inf arises via CPU/ALU arithmetic operations (e.g., 1 DIV 0) as

> opposed to assignment. In other words, if there were no instance of

> octave_ieee_init() in interpreter::interpreter, could there be situations

> where the non-IEEE float fails even when none of the

> abort-plus-informative-message checks happen and one simply gets

> abort-via-crash?

>

> And what about within lo-ieee.cc itself? There are currently no

> octave_ieee_init() within the routines

>

> __lo_ieee_is_NA (double x)

> __lo_ieee_is_old_NA (double x)

> __lo_ieee_float_is_NA (float x)

>

> Couldn't the user run a script, say "isinf(5)" or "isnan(20)", before

> actually creating a variable with value +/-Inf or NA?

>

> Dan

>

libinterp/corefcn/interpreter.cc, and removing the others.

but we probably want to preserve the flexibility to do so.

from the std library.