octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gplot.txt


From: Daniel J Sebald
Subject: Re: gplot.txt
Date: Fri, 04 Mar 2016 12:52:35 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.24) Gecko/20111108 Fedora/3.1.16-1.fc14 Thunderbird/3.1.16

On 03/04/2016 12:15 PM, Daniel J Sebald wrote:
On 03/04/2016 12:26 AM, Lachlan Andrew wrote:
On 4 March 2016 at 16:37, Daniel J Sebald<address@hidden> wrote:

assign memory even from the start--not a bad strategy. However, at that
point, the member pointers r and d are initialized as r(0) and d(0). And
2)
delete [] r;

Now, the first line of code is more questionable. Typically one isn't
supposed to delete a NULL pointer.

Good catch, but deleting NULL pointers is valid in all
standards-compliant compilers, and some people advocate that rather
than checking for NULL first. See the discussion at

http://stackoverflow.com/questions/4190703/is-it-safe-to-delete-a-null-pointer#4190737


Yes, appears to be fine.

It turns out that ::change_length() is accessed much more than I
thought. It is used in the header in inline function
Sparse::change_capacity (octave_idx_type nz), and that routine is used
at least a half dozen times in the source file.

Anyway, so the path that "speye(10)" follows through the instantiation
(Sparse::Sparse) is

else if (a_scalar)
{
[snip]
else
{
idx_vector rr = r;
idx_vector cc = c;
const octave_idx_type *rd = rr.raw ();
const octave_idx_type *cd = cc.raw ();

I did a deeper dive on the code in that path, and I too find it to be
fine in terms of indexing. It is a bit tricky to follow because the
array length is one longer than the number of nc, but I can't find any
place where an index (of index) might be one index out of bounds.

The one thing that has me a bit confused is this raw() function:

const octave_idx_type *
idx_vector::raw (void)
{
if (rep->idx_class () != class_vector)
*this = idx_vector (as_array (), extent (0));

idx_vector_rep * r = dynamic_cast<idx_vector_rep *> (rep);

assert (r != 0);

return r->get_data ();
}

Something else about the above routine is that a compiler has to be very careful about optimization. In other words, the above construct is something a compiler could have a bug with because it is tricky. This "copy" operation

  *this = idx_vector (as_array (), extent (0));

which I assume overwrites the existing memory for the "called" or "acting upon" object is inherently changing the value of "rep" pointer. So, by the time

  dynamic_cast<idx_vector_rep *> (rep);

takes place, "rep" has a new value. But what if an optimizing compiler doesn't understand that? The compiler might think "rep" is used here:

  if (rep->idx_class () != class_vector)

and then a few instructions later rep is used here:

  idx_vector_rep * r = dynamic_cast<idx_vector_rep *> (rep);

If the compiler were to keep "rep" local in some CPU register as a form of optimization rather than reloading, then "rep" is the old rep pointer, not the new rep pointer.

Dan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]