Can I replace this procedure with a function (VHDL 2008 problem)?

Discussion in 'VHDL' started by Tricky, May 9, 2014.

  1. Tricky

    Tricky Guest

    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 workout a way a function would work instead?
     
    Tricky, May 9, 2014
    #1
    1. Advertisements

  2. Tricky

    KJ Guest

    Actually you do know the length of the data field, it is start_of_list.data'length
    Below is a function that should be equivalent to your procedure. It compiles and accomplishes your goal of having a function, but whether it actuallyworks 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 theactual 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;
     
    KJ, May 9, 2014
    #2
    1. Advertisements

  3. Tricky

    Tricky Guest

    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 asa 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;
     
    Tricky, May 9, 2014
    #3

  4. 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, May 14, 2014
    #4
  5. Tricky

    Tricky Guest

    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, May 19, 2014
    #5
  6. Tricky

    Tricky Guest

    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, May 19, 2014
    #6
  7. Tricky

    Tricky Guest

    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;
     
    Tricky, May 19, 2014
    #7
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.