Can I replace this procedure with a function (VHDL 2008 prob

[Tricky]

This cropped up today while writing a testbench. Please look at the following code - it manages a linked list:

type queue is protected
procedure add_data(a : unsigned);
procedure get_data(a : out unsigned);
end protected queue;


type queue is protected body
type link_t;
type link_ptr_t is access link_t;

type link_t is record
data : unsigned;
next_link : link_ptr_t;
end record link_t;

variable start_of_list : link_ptr_t;

procedure add_data(a : unsigned) is
variable ptr : link_ptr_t := null;
begin
ptr := new link_t'( (a, start_of_list) );
start_of_list := ptr;
end procedure add_data;


procedure get_data(a : out unsigned) is
variable ptr : link_ptr_t := null;
begin
a := start_of_list.data;

ptr := start_of_list.next_link;
DEALLOCATE(start_of_list);

start_of_list := ptr;
end procedure get_data;

end protected body queue;

Ideally, the get_data procedure would actually be a function (as I have done in the past) but as the data field in the link_t is unconstrained in the type, I cannot create a temporary variable to place the data into before returning it from the function, as I dont know the length of the data field before pulling it out of the list.

Would the above be the best way of doing it, or can some others of you work out a way a function would work instead?

 

[KJ]

On Friday, May 9, 2014 9:38:58 AM UTC-4, Tricky wrote:

This cropped up today while writing a testbench. Please look at the following
Ideally, the get_data procedure would actually be a function (as I have done
in the past) but as the data field in the link_t is unconstrained in the
type, I cannot create a temporary variable to place the data into before
returning it from the function, as I dont know the length of the data field
before pulling it out of the list.

Actually you do know the length of the data field, it is start_of_list.data'length

Would the above be the best way of doing it, or can some others of you work
out a way a function would work instead?

Below is a function that should be equivalent to your procedure. It compiles and accomplishes your goal of having a function, but whether it actually works or not I didn't test.

I kind of have my doubts since the 'data' element of your record is not constrained it's not immediately obvious where you're allocating space for the actual data element when you go to 'put' the data. I'm guessing that although it compiles, neither your procedure nor my function will actually run. But in any case, I believe that what I have posted for the function is the equivalent to your procedure.

Kevin Jennings

impure function get_data2 return unsigned is
variable ptr : link_ptr_t := null;
variable a: unsigned(start_of_list.data'range);
begin
a := start_of_list.data;

ptr := start_of_list.next_link;
DEALLOCATE(start_of_list);

start_of_list := ptr;
return(a);
end function get_data2;

 

[Tricky]

This is fine, but if the start_of_list is null, you will get an error from a null pointer access.

In the real code I have a record type instead of just an unsigned. If the start_of_list is null it returns a NULL version of the record type I have as a constant. I actually worked around the problem by creating a pointer to the record type, pulling the data off the linked list into the pointer, then returning the de-referenced data:

impure function get_packet return packet_info_t is
variable ptr : packet_ll_ptr_t := null;
variable ret : packet_info_ptr_t := null;
begin
if end_of_list = null then
ret := new packet_info_t'(NULL_PACKET);

else
ptr := end_of_list.next_packet;
ret := new packet_info_t'( ptr.pi );

DEALLOCATE(end_of_list);
end_of_list := ptr;

end if;

return ret.all;
end function get_packet;

On Friday, 9 May 2014 15:57:44 UTC+1, KJ wrote:

On Friday, May 9, 2014 9:38:58 AM UTC-4, Tricky wrote:

This cropped up today while writing a testbench. Please look at the following

Ideally, the get_data procedure would actually be a function (as I have done

in the past) but as the data field in the link_t is unconstrained in the

type, I cannot create a temporary variable to place the data into before

returning it from the function, as I dont know the length of the data field

before pulling it out of the list.



Actually you do know the length of the data field, it is start_of_list.data'length





Would the above be the best way of doing it, or can some others of you work

out a way a function would work instead?



Below is a function that should be equivalent to your procedure. It compiles and accomplishes your goal of having a function, but whether it actually works or not I didn't test.



I kind of have my doubts since the 'data' element of your record is not constrained it's not immediately obvious where you're allocating space for the actual data element when you go to 'put' the data. I'm guessing that although it compiles, neither your procedure nor my function will actually run. But in any case, I believe that what I have posted for the function is the equivalent to your procedure.



Kevin Jennings



impure function get_data2 return unsigned is

variable ptr : link_ptr_t := null;

variable a: unsigned(start_of_list.data'range);

begin

a := start_of_list.data;



ptr := start_of_list.next_link;

DEALLOCATE(start_of_list);



start_of_list := ptr;

return(a);

end function get_data2;

 

[Paul Uiterlinden]

Tricky wrote:

This is fine, but if the start_of_list is null, you will get an error from
a null pointer access.

In the real code I have a record type instead of just an unsigned. If the
start_of_list is null it returns a NULL version of the record type I have
as a constant. I actually worked around the problem by creating a pointer
to the record type, pulling the data off the linked list into the pointer,
then returning the de-referenced data:

impure function get_packet return packet_info_t is
variable ptr : packet_ll_ptr_t := null;
variable ret : packet_info_ptr_t := null;
begin
if end_of_list = null then
ret := new packet_info_t'(NULL_PACKET);

else
ptr := end_of_list.next_packet;
ret := new packet_info_t'( ptr.pi );

DEALLOCATE(end_of_list);
end_of_list := ptr;

end if;

return ret.all;
end function get_packet;

You create a memory leak in this way, so don't do it this way.

Every time get_packet is called, a packet is allocated, never deallocated
and the pointer to it (ret) is lost upon exiting the function. VHDL does
not know the concept of garbage collection, so this is a memory leak.

To verify this, put the get_packet in a loop, endless, or a lot of times
(increasing it tenfold with each run) and observe the memory usage of your
machine. Or, in case of the endless loop, watch the simulator crash (and/or
see you machine getting irresponsive).

To get around the unconstrained issue, I would put the code in a package.
Put a generic on that package, specifying the width of the data. Or better
still, put the datatype in the generic of the package!

When instantiating the package, you specify the datatype. In that way you
can use your queue for any data type.

Check out the book "VHDL-2008, just the new stuff" by Peter Ashenden and Jim
Lewis. It is all described there, in the first chapter.

One more thing: when declaring a variable of an access type, it is not
necessary to initialize it to null. The initial value of variables and
signals automatically is "the most left value" of the type of the variable
or signal. For natural it is 0, for boolean false, for enumeration types
the first member, for access types it is null.

People sometimes complain that VHDL is overly verbose, but at the same time
they type needless things... ;-)

--
Paul Uiterlinden
www.aimvalley.nl

 

[Tricky]

You create a memory leak in this way, so don't do it this way.

The memory leak is a concern, but this is best way around my problem.

I cannot put the type in a package, because the the packet length is variable at run time. The whole point is the packet generator generates variable length packets for the UUT to handle. So even the package generics do not help here.

Because of this, I do not know the length of the packet until it is pulled out of the queue, so I cannot create a temporary variable -----

Actually, I just thought, it wouldnt be very hard just to access the length of the packet on the end of the queue to return the length to create the temporary variable, so it is not a pointer.

 

[Tricky]

So all I needed was an extra function to check if the end of queue was already null, or just return the length:

------------------------------------------------------------------
--Removes a packet from the FIFO Queue
------------------------------------------------------------------
impure function get_packet return packet_info_t is

impure function get_payload_len return integer is
begin
if end_of_queue = null then
return 0;
else
return end_of_queue.payload'length;
end if;
end function get_payload_len;

variable ptr : packet_ll_ptr_t := null;
variable ret : packet_info_t( payload(0 to get_payload_len-1));
begin
if end_of_list = null then
ret := NULL_PACKET;

else
ret := end_of_list.pi;
ptr := end_of_list.next_packet;

DEALLOCATE(end_of_list);
end_of_list := ptr;

-------------------------------------------------------------------------------------------
--The list has been emptied, so to prevent a null pointer reference, reset the start of
--list too to prevent a dangling pointer
-------------------------------------------------------------------------------------------
if end_of_list = null then

start_of_list := null;
end if;

end if;

return ret;
end function get_packet;

 

[Tricky]

Doh must remember to syntax check first!

------------------------------------------------------------------
--Removes a packet from the FIFO Queue
------------------------------------------------------------------
impure function get_packet return packet_info_t is

impure function get_payload_len return integer is
begin
if end_of_list= null then
return 0;
else
return end_of_list.pi.payload'length;
end if;
end function get_payload_len;

variable ptr : packet_ll_ptr_t := null;
variable ret : packet_info_t( payload(0 to get_payload_len-1));
begin
if end_of_list = null then
ret := NULL_PACKET;

else
ret := end_of_list.pi;
ptr := end_of_list.next_packet;

DEALLOCATE(end_of_list);
end_of_list := ptr;

-------------------------------------------------------------------------------------------
--The list has been emptied, so to prevent a null pointer reference, reset the start of
--list too to prevent a dangling pointer
-------------------------------------------------------------------------------------------
if end_of_list = null then

start_of_list := null;
end if;

end if;

return ret;
end function get_packet;

 

[Jim Lewis]

A very latent reply, however, in my scoreboard that does this, I keep a previous item pointer that I deallocate on entering pop and leave the previous item in when I exit.