Monday, November 17, 2014

Experimental Cures for Flattened Register Definitions in vr_ad

On my current project, I had an issue with my register definitions. Quite a few of my DUT's registers where just instances of the same register type. My vr_ad register definitions were generated by a script, based on the specification, a flow that I'm pretty sure is very similar to what most of you also have. Instead of generating a nice regular structure, this script created a separate type for each register instance. What resulted was a flattened structure where I'd, for example, get one instance each of registers SOME_REG0, SOME_REG1, SOME_REG2, instead of three instances of SOME_REG. I was lucky enough to be able to (partly) change the definitions by patching them by hand.

Someone on StackOverflow had the same problem, but didn't have the luxury of being able to fix it like I did. They weren't allowed to touch the code as I'm guessing it probably belonged to a different team. They probably also had a lot of legacy code that was using those flattened register definitions. This made me want to do an experimental post on how to best cope with such an issue.

Naturally the best thing to do is to fix the underlying problem of the registers getting flattened, but that might not be possible, so let's look at how to fix the symptoms.

To be able to do any kind of serious modeling, we need to be able to program generically. We can't (easily) do this if each register is an own type. I've tried to think of how to best handle this from a maintainability point of view. As a bonus requirement, we'd also like it that when the register definitions do get fixed (i.e. the generation flow gets updated) we have to make as few changes as possible to the modeling code.

Enough with the stories, let's get our hands dirty. As always, we'll start small, but think big. We'll go through a few iterations, look at where we're lacking and gradually refine our approach.

Let's say we have a device that can operate with shapes. Part of its functionality involves doing stuff with triangles. It can process multiple triangles at the same time, where each triangle is described by a register containing the lengths of its sides. Our DUT does computations on the triangles, based on these values. For example, it can compute the areas of the triangles. We want to check that what the DUT writes out is correct so we need to model these computations.

We have a trusty script that can generate the register definitions from the specification (maybe an XML file). This script isn't very well written and it doesn't know that all three TRIANGLE registers are just the same register instantiated 3 times (i.e. a regular structure), or maybe the information got lost in the XML somehow. This is what we get for our register definitions:

<'
extend vr_ad_reg_file_kind : [ GRAPHICS ];
extend GRAPHICS vr_ad_reg_file {
  keep size == 256;
  post_generate() is also {
    reset();
  };
};

reg_def TRIANGLE0 GRAPHICS 0x00 {
  reg_fld SIDE0 : uint(bits : 8);
  reg_fld SIDE1 : uint(bits : 8);
  reg_fld SIDE2 : uint(bits : 8);
};

reg_def TRIANGLE1 GRAPHICS 0x10 {
  reg_fld SIDE0 : uint(bits : 8);
  reg_fld SIDE1 : uint(bits : 8);
  reg_fld SIDE2 : uint(bits : 8);
};

reg_def TRIANGLE2 GRAPHICS 0x20 {
  reg_fld SIDE0 : uint(bits : 8);
  reg_fld SIDE1 : uint(bits : 8);
  reg_fld SIDE2 : uint(bits : 8);
};
'>

Our reference model will contain a pointer to the register file:

<'
struct flattened_graphics_model {
  graphics_regs : GRAPHICS vr_ad_reg_file;
};
'>

The reference model needs to be able to compute the area of each triangle. As a first idea, we create a method for each triangle that implements Heron's formula:

<'
extend flattened_graphics_model {
  get_triangle0_area() : real is {
    var triangle0 := graphics_regs.triangle0;
    var half_per : real = 0.5 *
      (triangle0.SIDE0 + triangle0.SIDE1 + triangle0.SIDE2);
    result = sqrt(
      (half_per - triangle0.SIDE0) *
      (half_per - triangle0.SIDE1) *
      (half_per - triangle0.SIDE2) *
      half_per
    );
  };
  
  get_triangle1_area() : real is {
    var triangle1 := graphics_regs.triangle1;
    var half_per : real = 0.5 *
      (triangle1.SIDE0 + triangle1.SIDE1 + triangle1.SIDE2);
    result = sqrt(
      (half_per - triangle1.SIDE0) *
      (half_per - triangle1.SIDE1) *
      (half_per - triangle1.SIDE2) *
      half_per
    );
  };
  
  get_triangle2_area() : real is {
    var triangle2 := graphics_regs.triangle2;
    var half_per : real = 0.5 *
      (triangle2.SIDE0 + triangle2.SIDE1 + triangle2.SIDE2);
    result = sqrt(
      (half_per - triangle2.SIDE0) *
      (half_per - triangle2.SIDE1) *
      (half_per - triangle2.SIDE2) *
      half_per
    );
  };
};
'>

We can immediately see a problem with this approach. We've implemented the formula in three different places. This means that should something change, we have three places to fix. Now, Heron's formula changing is a pretty unlikely event, but should we have a different computation to perform here the discussion stands.

What we can do is extract the part that computes the actual area as an own method, that takes the three sides as its arguments:

<'
get_triangle_area(side0 : uint, side1 : uint, side2 : uint) : real is {
  var half_per : real = 0.5 * (side0 + side1 + side2);
  result = sqrt(
    (half_per - side0) *
    (half_per - side1) *
    (half_per - side2) *
    half_per
  );
};
'>

We can simplify the three methods from before to just call this generic method:

<'
get_triangle0_area() : real is {
  var triangle := graphics_regs.triangle0;
  result = get_triangle_area(triangle.SIDE0, triangle.SIDE1,
    triangle.SIDE2);
};

get_triangle1_area() : real is {
  var triangle := graphics_regs.triangle1;
  result = get_triangle_area(triangle.SIDE0, triangle.SIDE1,
    triangle.SIDE2);
};

get_triangle2_area() : real is {
  var triangle := graphics_regs.triangle2;
  result = get_triangle_area(triangle.SIDE0, triangle.SIDE1,
    triangle.SIDE2);
};
'>

At least this way we've centralized the computation part to one location. The number of such methods will grow linearly, though, with the number of TRIANGLE registers. This means that for n triangles we'll need n methods to compute the areas.

Let's add a new requirement: our DUT is also able to compute which triangle is the largest and we need to model that too. We can define a new method to do that based on the areas:

<'
largest() : uint is {
  var areas : list of real;
  areas.add(get_triangle0_area());
  areas.add(get_triangle1_area());
  areas.add(get_triangle2_area());
  
  result = areas.max_index(it);
};
'>

In this method, the number of calls to get_triangleX_area() also grows with the number of triangles. Moreover, if we want to be able to find out which triangle is the smallest, the method for that would have to look like this:

<'
smallest() : uint is {
  var areas : list of real;
  areas.add(get_triangle0_area());
  areas.add(get_triangle1_area());
  areas.add(get_triangle2_area());
  
  result = areas.min_index(it);
};
'>

Pretty much the same as largest(), isn't it? In this setup, adding a single triangle would require adding a new method for the area and changing two others. That's not very maintainable. We can use the same trick we did for the area computation and pull out computing the list of areas to it's own method, while simplifying the largest() and smallest() methods:

<'
get_triangle_areas() : list of real is {
  result.add(get_triangle0_area());
  result.add(get_triangle1_area());
  result.add(get_triangle2_area());
};

largest() : uint is {
  var areas := get_triangle_areas();
  result = areas.max_index(it);
};

smallest() : uint is {
  var areas := get_triangle_areas();
  result = areas.min_index(it);
};
'>

Now we only need to update the get_triangle_areas() method when adding a new triangle. Not much of an improvement, but every little thing counts when you're potentially dealing with a large number of triangles.

While we may have things sorted out for areas, we get a new requirement. Our DUT can also compute perimeters and tell us which triangle is the longest and which one is the shortest. This means we'll need to add a similar set of methods to handle this aspect, based on the examples from above:

<'
extend flattened_graphics_model {
  get_triangle_perimeter(side0 : uint, side1 : uint, side2 : uint) : uint is {
    result = side0 + side1 + side2;
  };
  
  get_triangle0_perimeter() : uint is {
    var triangle := graphics_regs.triangle0;
    result = get_triangle_perimeter(triangle.SIDE0, triangle.SIDE1,
      triangle.SIDE2);
  };
  
  get_triangle1_perimeter() : uint is {
    var triangle := graphics_regs.triangle1;
    result = get_triangle_perimeter(triangle.SIDE0, triangle.SIDE1,
      triangle.SIDE2);
  };
  
  get_triangle2_perimeter() : uint is {
    var triangle := graphics_regs.triangle2;
    result = get_triangle_perimeter(triangle.SIDE0, triangle.SIDE1,
      triangle.SIDE2);
  };
  
  get_triangle_perimeters() : list of uint is {
    result.add(get_triangle0_perimeter());
    result.add(get_triangle1_perimeter());
    result.add(get_triangle2_perimeter());
  };
  
  longest() : uint is {
    var perimeters := get_triangle_perimeters();
    result = perimeters.max_index(it);
  };
  
  shortest() : uint is {
    var perimeters := get_triangle_perimeters();
    result = perimeters.min_index(it);
  };
};
'>

Adding just one measly triangle is starting to become a real pain. What would be awesome is being able to just add one line of code every time a new triangle gets added and be done with it. Well, thanks to our good friends, the macros, this is possible.

What we notice is that the code is very regular. Aside from the indices, the method bodies look remarkably similar. This means that for the area aspect we can create the following macro:

<'
define <triangle_area_utils'statement> "triangle_area_utils <num>" as {
  extend flattened_graphics_model {
    get_triangle<num>_area() : real is {
      var triangle := graphics_regs.triangle<num>;
      result = get_triangle_area(triangle.SIDE0, triangle.SIDE1,
        triangle.SIDE2);
    };
    
    get_triangle_areas() : list of real is also {
      result.add(get_triangle<num>_area());
    };
  };
};
'>

Adding a new triangle is now as easy as just expanding the macro with the appropriate argument:

<'
triangle_area_utils 0;
triangle_area_utils 1;
triangle_area_utils 2;
'>

We could define a similar macro for the perimeter aspect (I won't show it here). While we have made adding new triangles easier, we've also shot ourselves in the foot. Excessive use of macros is a code smell because it can be very difficult to understand what code gets expanded in the background. Also, it makes the code more difficult to refactor, since we can't rely on fancy IDE features.

If we analyze the code up now we see that one of our main problems is that each triangle is stored in an individual field. This means that there's no way to access a triangle from a method by just passing in the index of the triangle (0, 1, 2, etc.). If we could do this, we could get rid of all our get_triangleX_area() methods.

A way of doing this is using the reflection API. Reflection allows us, among others, to get a field of a struct by using only the name of that field, specified as a string. In our case, we know that our register file contains fields named triangle0, triangle1, triangle2, etc. We can use the reflection API to extract the field that contains contains the appropriate index as its suffix:

<'
extend flattened_graphics_model {
  num_triangles : uint;
    keep num_triangles == 3;
  
  get_triangle_reg(idx : uint) : vr_ad_reg is {
    assert idx < num_triangles;
    
    var regs_type := rf_manager.get_exact_subtype_of_instance(graphics_regs);
    
    var triangle_reg_field :=
      regs_type.get_fields().first(it.get_name() == appendf("triangle%d", idx));
    assert triangle_reg_field != NULL;
    
    assert triangle_reg_field.get_type() ==
      rf_manager.get_type_by_name(appendf("TRIANGLE%d'kind vr_ad_reg", idx));
    result =
      triangle_reg_field.get_value(graphics_regs).get_value().unsafe();
  };
};
'>

The way to use the reflection API is to get the representation of our register file from the rf_manager singleton. What we'll end up with is a struct of type rf_struct that understands what fields, methods, etc. the register file has. Out of this we can extract a representation of the field for the triangle that interests us, of type rf_field. Based on this field we can construct our return value. How exactly this happens is explained in the documentation and in this excellent post from the Specman R&D team. Have a look at those resources for more details on how to use the reflection interface.

After we've gotten an instance of our desired register, we can use this to compute the area. We can do away with the get_triangleX_area() methods and replace them with one get_triangle_area_by_index(...) method:

<'
get_triangle_area_by_index(idx : uint) : real is {
  assert idx < num_triangles;
  var reg := get_triangle_reg(idx);
  var reg_type := rf_manager.get_exact_subtype_of_instance(reg);
  
  var side0_field := reg_type.get_fields().first(it.get_name() == "SIDE0");
  assert side0_field != NULL;
  assert side0_field.get_type() == rf_manager.get_type_by_name("uint(bits:8)");
  var side0 : uint = side0_field.get_value(reg).get_value().unsafe();
  
  var side1_field := reg_type.get_fields().first(it.get_name() == "SIDE1");
  assert side1_field != NULL;
  assert side1_field.get_type() == rf_manager.get_type_by_name("uint(bits:8)");
  var side1 : uint = side1_field.get_value(reg).get_value().unsafe();
  
  var side2_field := reg_type.get_fields().first(it.get_name() == "SIDE2");
  assert side2_field != NULL;
  assert side2_field.get_type() == rf_manager.get_type_by_name("uint(bits:8)");
  var side2 : uint = side2_field.get_value(reg).get_value().unsafe();
  
  result = get_triangle_area(side0, side1, side2);
};
'>

Because the return value of get_triangle_reg(...) is of type vr_ad_reg, we can't reference the SIDEx fields directly (as these are defined under when subtypes). We can't cast the value to any of these subtypes, because we would need n cast statements (the very thing we want to avoid). We can use the same method as before to get the values of the sides via the reflection interface. The resulting code isn't pretty, but it works. Can we do better, though?

Of course we can! An essential observation to make here is that all triangle register types contain the same fields, whether they are of type TRIANGLE0 or TRIANGLE1 or TRIANGLE2. We could do all of our operations using only a variable of one of these types, provided that we fill it up with the appropriate values for the sides. That is, a TRIANGLE0 with sides 1, 2 and 3 has the same area as a TRIANGLE1 with the same sides. With this idea in mind, we can do the following:

<'
get_triangle_area_by_index(idx : uint) : real is {
  assert idx < num_triangles;
  var triangle : TRIANGLE0 vr_ad_reg = new;
  triangle.write_reg_rawval(get_triangle_reg(idx).read_reg_rawval());
  result = get_triangle_area(triangle.SIDE0, triangle.SIDE1,
    triangle.SIDE2);
};
'>

We can just create a variable of type TRIANGLE0 and fill it up with the contents of our desired register. We can then reference the SIDE fields directly, without the need for all of that messy reflection code. The price we pay for this convenience, however is in essence a copy operation. Whether this is slower than using the reflection interface I can't say (though I suspect it isn't), but it is in any case cleaner.

Our largest() method becomes pretty trivial to write:

<'
largest() : uint is {
  var areas : list of real;
  for i from 0 to num_triangles - 1 {
    areas.add(get_triangle_area_by_index(i));
  };
  
  result = areas.max_index(it);
};
'>

Not only that, but we can now handle any number of triangles without increasing the number of lines in the code. The only modification we need to make is to set the num_triangles field to the appropriate value.

I'd propose one final refactoring step. Why do we have to define the methods that compute the area and the perimeter inside the reference model? A triangle register contains all of the information required to compute these values. Seeing as how we'll just be using the TRIANGLE0 subtype in our code, we can extend that to contain a get_area() method:

<'
extend TRIANGLE0 vr_ad_reg {
  get_area() : real is {
    var half_per : real = 0.5 * (SIDE0 + SIDE1 + SIDE2);
    result = sqrt(
      (half_per - SIDE0) *
      (half_per - SIDE1) *
      (half_per - SIDE2) *
      half_per
    );
  };
};
'>

Getting the area of a triangle becomes just:

<'
print graphics_model.get_triangle_reg(0).get_area();
'>

We can also rewrite the largest() method as:

<'
extend flattened_graphics_model {
  get_triangle_regs() : list of TRIANGLE0 vr_ad_reg is {
    for i from 0 to num_triangles - 1 {
      result.add(get_triangle_reg(i));
    };
  };
  
  largest() : uint is {
    var triangles := get_triangle_regs();
    result = triangles.max_index(it.get_area());
  };
};
'>

Of course, we can do the same for the perimeter aspect (not shown here). Let's take a moment to see what we've achieved. We've managed to program our computations in a generic way, by relying on methods that take the index of a register as a parameter. This saves us a lot of typing because we don't have to define a method that accesses each field. We've also nicely encapsulated our methods: all methods that refer to a single triangle (get_area() and get_perimeter()) are defined in the triangle register struct, while the methods that refer to all triangles are encapsulated in the reference model struct.

Further above, I've mentioned the bonus requirement that we want our resulting code to look as similar as possible to the case where the register definitions aren't flattened. Let's see how our reference model would look in the ideal case.

First we have to start with our register definitions:

<'
reg_def TRIANGLE {
  reg_fld SIDE0 : uint(bits : 8);
  reg_fld SIDE1 : uint(bits : 8);
  reg_fld SIDE2 : uint(bits : 8);
};


extend GRAPHICS vr_ad_reg_file {
  triangles[3] : list of TRIANGLE vr_ad_reg;
  
  add_registers() is also {
    for each (triangle) in triangles {
      add_with_offset(index * 0x10, triangle);
    };
  };
};
'>

Since there is only one triangle struct, we extend that to add the get_area() method:

<'
extend TRIANGLE vr_ad_reg {
  get_area() : real is {
    var half_per : real = 0.5 * (SIDE0 + SIDE1 + SIDE2);
    result = sqrt(
      (half_per - SIDE0) *
      (half_per - SIDE1) *
      (half_per - SIDE2) *
      half_per
    );
  };
};
'>

Finding the largest and the smallest triangles is easily done by iterating over the triangles list of the register file:

<'
extend compacted_graphics_model {
  largest() : uint is {
    var triangles := graphics_regs.triangles;
    result = triangles.max_index(it.get_area());
  };
  
  smallest() : uint is {
    var triangles := graphics_regs.triangles;
    result = triangles.min_index(it.get_area());
  };
};
'>

Notice that we don't need the get_triangle_regs() method anymore, as we already have our triangles organized in a list. If we were to implement the last proposal, once our register definitions would be fixed, migrating to the new structure would only require some minor search and replace operations. This goes to show that starting off on the wrong foot doesn't mean we're completely out of the dance. With some extra work, we can get very close to the ideal solution, but we have to be willing to compromise a bit on simulation speed. Still, it's better than compromising on maintainability and getting stuck in an endless loop of bad coding style.

I hope you found this post useful. I've posted the code to SourceForge for reference. Stay tuned for more!

Saturday, November 1, 2014

Using indirect_access(...) in vr_ad

I've been working a lot with vr_ad lately. It has a lot of nice features for modeling registers, but unfortunately not all of them are documented. I'm going to do a longer series of posts based on my recent experiences.

Let's start out small. We've all had the case where accesses to one register of the design affect the values of other registers. This might be best illustrated with a concrete example. Let's say we have a status register defined as follows:

<'
reg_def STATUS EXAMPLE 0x0 {
  reg_fld VALID : uint(bits : 1) : R : 0x0;
  reg_fld DONE  : uint(bits : 1) : R : 0x0;
};
'>

We'll also have a control register:

<'
reg_def CONTROL EXAMPLE 0x4 {
  reg_fld SETVALID : uint(bits : 1) : W : 0x0;
  reg_fld CLRVALID : uint(bits : 1) : W : 0x0;
  reg_fld CLRDONE  : uint(bits : 1) : W : 0x0;
  reg_fld START    : uint(bits : 1) : W : 0x0;
};
'>

The status flags are updated by the device. Using the control register's SETVALID and CLRVALID fields, we can affect the value of the VALID status flag. Whenever our VALID flag is set, our DUT can start crunching data. The operation is triggered by writing the START field of the control register and terminates within 5 clock cycles. The DONE status flag is set by the hardware once the operation completes and can be cleared by writing to the CLRDONE control field.

In the past I implemented this by giving the affecting register pointers to the affected registers and implementing the logic inside the post_access(...) method. While this works, vr_ad provides a neater way of doing it.

What we first need to do is to mark the status register as an observer of the control register:

<'
extend EXAMPLE vr_ad_reg_file {
  add_registers() is also {
    control.attach(status);
  };
};
'>

Now, whenever the control register is accessed, a method called indirect_access(...) is called inside the status register. This method receives the direction of the access, together with the observed vr_ad object (in our case it's a register, but it could just as well be a register file, a memory, etc.). Using this information we can model the evolution of the status flags.

Let's start with modeling the VALID flag. As stated above, writing a '1' to SETVALID will set the flag, while writing a '1' to CLRVALID will clear it. Simultaneously writing '1's to both fields doesn't make sense, so what we'll do in that case is leave the flag unchanged. Let's distil this behavior into a method:

<'
extend STATUS vr_ad_reg {
  model_valid(control : CONTROL vr_ad_reg) is {
    if control.SETVALID == 1 and control.CLRVALID == 0 {
      VALID = 1;
    }
    else if control.CLRVALID == 1 {
      VALID = 0;
    };
  };
};
'>

We'll call this method from within indirect_access(...), whenever a write to CONTROL happens:

<'
extend STATUS vr_ad_reg {
  indirect_access(direction : vr_ad_rw_t, ad_item : vr_ad_base) is {
    if direction == WRITE {
      var control := ad_item.as_a(CONTROL vr_ad_reg);
      assert control != NULL;
      
      model_valid(control);
    };
  };
};
'>

Let's give it a test drive to make sure that everything works. We'll emulate a monitor updating the register model by calling update(...) for writes and compare_and_update(...) for reads. To make things easier, let's wrap these calls into two handy methods to access the registers:

<'
extend sys {
  reg_file : EXAMPLE vr_ad_reg_file;
  
  event clk is @sys.any;
  
  write_control(data : vr_ad_data_t) @clk is {
    reg_file.update(0x4, pack(packing.high, data), {});
    wait [1];
  };
  
  read_status(data : vr_ad_data_t) @clk is {
    compute reg_file.compare_and_update(0x0, pack(packing.high, data));
    wait [1];
  };
};
'>

The data argument we pass to these methods represents the data seen on the bus. When reading the status register, if the data we pass to compare_and_update(...) doesn't match the model's value, an error message will appear and we'll know we've made a mistake.

Let's start by trying to set VALID:

<'
extend sys {
  run() is also {
    start do_test();
  };
  
  do_test() @clk is {
    // set VALID
    write_control(0b1000);
    
    // expect to read VALID
    read_status(0b10);
  };
};
'>

Let's also try to clear VALID:

<'
extend sys {
  do_test() @clk is also {
    // clear VALID
    write_control(0b0100);
    
    // expect to read not VALID
    read_status(0b00);
  };
};
'>

Let's tackle the DONE flag now. We'll need to define a clock event to handle the duration of the operation. Also, our modeling method will have to be a TCM:

<'
extend STATUS vr_ad_reg {
  event clk;
  
  model_done(control : CONTROL vr_ad_reg) @clk is {
    if control.CLRDONE == 1 {
      DONE = 0;
    };
    
    if control.START == 1 and VALID == 1 {
      wait [5];
      DONE = 1;
    };
  };
'>

We'll start this method from indirect_access(...):

<'
extend STATUS vr_ad_reg {
  indirect_access(direction : vr_ad_rw_t, ad_item : vr_ad_base) is {
    if direction == WRITE {
      // ...
      
      start model_done(control);
    };
  };
};
'>

As before, let's test this to make sure it works. First let's set VALID and issue a START command. With an immediate read from the status register we'll only expect to see the VALID flag set. After 5 clock cycle we'll expect to see both VALID and DONE set:

<'
extend sys {
  do_test() @clk is also {
    // set VALID and START
    write_control(0b1001);
    
    // expect to read VALID
    read_status(0b10);
    
    // after 5 clocks, expect to read DONE as well
    for i from 1 to 5 { wait [1] };
    read_status(0b11);
  };
};
'>

After writing CLRDONE, we expect to see the DONE flag cleared:

<'
extend sys {
  do_test @clk is also {
    // clear DONE
    write_control(0b0010);
    
    // expect to read not DONE
    read_status(0b10);
  };
};
'>

And there we have it: a nice, clean way of modeling register interdependencies. This pattern can be extended to however many other registers might depend on the CONTROL register. Best of all, it encapsulates the modeling logic inside the register that is being affected, as opposed to inside the affecting register. This means that we can easily add and remove observer registers as we please, as they are all independent of each other.

You can find the complete code on SourceForge if you want to try it out.

Have fun with your register modeling!

P.S.

Don't forget to subscribe if you don't want to miss out on any future vr_ad related posts. You can also be notified about new posts via email by using the "Subscribe by Email" box.

Tuesday, October 7, 2014

A New Twist on SystemVerilog Enumerated Types

A well known SystemVerilog limitation is that the same literal cannot appear in more enumerated types within a package (or more precisely within a scope).

Let's look at a concrete example. We'll assume that we're verifying a DUT that can receive data from the outside world, perform some mathematical operations on it and sends it back. We want to model the operations that our DUT performs and the best way to do that is by using two enumerated types:

package my_pkg;

  typedef enum { NONE, TX, RX } comm_action_t;
  typedef enum { NONE, ADD, SUB } math_action_t;
  
  // code that uses the enums
  // ...
endpackage

The DUT doesn't continuously crunch data, so we want to add a literal for each enum to represent it not doing anything. Let's use the value NONE (I know that for math operations the value NOP would have been more appropriate, but please bear with me, I'm trying to illustrate a point). As discussed above, this code won't compile, because NONE is declared in both types.

What I've seen people do in this case is try to uniquify the names by adding either prefixes or suffixes. I, too, plead guilty to this. For example, the math_action_t type would contain the value NONE2, in order not to clash with the NONE from comm_action_t. This solution seems clumsy to me. Not only that, but if you're trying to connect to a VHDL DUT one "Big Three" simulator is going to complain because the literals don't exactly match (note: VHDL allows the same literal to be present in multiple types).

A very naïve solution would be to define each type in its own package. In the main package we would then import both of these packages:

package pkg1;
  typedef enum { NONE, TX, RX } comm_action_t;
endpackage

package pkg2;
  typedef enum { NONE, ADD, SUB } math_action_t;
endpackage


package my_pkg;
  import pkg1::*;
  import pkg2::*;
  
  class model;
    comm_action_t comm_action;
    math_action_t math_action;
    
    // ...
  endclass
endpackage

We've solved the collision problem, because each type is now defined in its own scope. We can have our cake and eat it too! Or can we? Let's see what happens if we try to use the NONE literal in some procedural code:

class model;
  // ...
  
  function new();    
    comm_action = NONE;
    math_action = NONE;
  endfunction
endclass

Your simulator should, at this point, cowardly refuse to compile the code above, because the literal NONE was imported via wildcards multiple times and it's ambiguous. The correct way to do it is to qualify it with the appropriate packages:

class model;
  // ...
  
  function new();    
    comm_action = pkg1::NONE;
    math_action = pkg2::NONE;
  endfunction
endclass

This solution works as well as the first one. In some respects it's more elegant, but it's also clumsier. Creating a package for each enum will pollute our work library. Also, when using literals in our code, the values ADD and TX, for example, can be written as-is, but we have to write pkg1::NONE. This isn't uniform at all. In addition, think of what would happen if we had to add a new type to a third package that doubled up the value ADD. We'd have to go and scope all of the existing references to ADD with pkg2.

Let's take a step back and consider another situation. What if we want to model each area of functionality separately? What I mean is, instead of creating a big model class that can handle everything, let's assume that we can create a clean split between the communication side and the math side. In this case, each model would be an own class. This means that each class can just embed the enumerated variable definition:

class comm_model;
  enum { NONE, TX, RX } comm_action;
  
  // ...
endclass


class math_model;
  enum { NONE, ADD, SUB } math_action;
  
  // ...
endclass

Since each class represents a different scope, we don't have any problem with the definition of NONE. What we defined here for comm_model, for example, is just a variable called comm_action that can take any of the three values. We can use this as we would any enumerated variable. Hey, you know what else we can define inside a class? An actual type. I guess you already know why I made this little detour...

What if we mixed the two approaches and just defined each type in its own class instead of in its own package? Here's how this would look like:

package my_pkg;
  virtual class comm_action_wrap;
    typedef enum { NONE, TX, RX } t;
  endclass
  
  virtual class math_action_wrap;
    typedef enum { NONE, ADD, SUB } t;
  endclass
  
  // ...
endpackage

Again, since each class is its own scope, we don't have any problem with collisions. Also, to prevent anyone from instantiating these classes, we define them virtual; their only purpose is to wrap the type definitions. If we want to use these wrapped enumerations, we have to scope them with their containing classes:

class model;
  comm_action_wrap::t comm_action;
  math_action_wrap::t math_action;
  
  function new();
    comm_action = comm_action_wrap::NONE;
    math_action = math_action_wrap::NONE;
  endfunction
    
  // ...
endclass

Notice that we also need to scope the enum literals. This solution is more uniform, in that it treats all literals the same (we have to scope all of them). It's also better encapsulated, since we only create extra class types inside our own package. It is, of course, more verbose, but we can't getting something for nothing.

C++ had the same problem with enumerated types, but the C++11 standard fixed that by adding a new construct, the enum class. This is basically the same thing as we just saw above, but it is a first class construct in the language. If you're curious about the topic, you can read more about it here.

I for one would like to see a similar enhancement to SystemVerilog in the future. What I would avoid, however, is calling it an enum class, as I think the word "class" carries a different connotation and would just confuse users (we anyway have the problem that the word "interface" has a double meaning). What might be practical though is to add scoping to the current construct and to allow the compiler to determine the type of a literal based on the context (à la VHDL or e). Here's what I mean:

typedef enum { NONE, TX, RX } comm_action_t;

class some_class;
  function void do_stuff();
    comm_action_t comm_action = TX;
    // ...
    
    if (comm_action == RX) begin
      // ...
    end
  endfunction
  
  function void do_other_stuff();
    int some_int = int'(comm_action_t::NONE);
  endfunction
endclass

In the code snippet above, when assigning a value to comm_action we can just omit the scope operator, because from the context we know that the left hand side of the expression is of type comm_action_t, so we would expect the right hand side to be of the same type. The situation is similar for the logical operator inside the if statement. If however we want to do a cast, we can't figure out from the context what enum the value NONE belongs to (as it could be multiply defined), so we have to use the :: operator. This solution would mean that code written in SV2012 could potentially be incompatible to the SV2099 version (that includes this proposal), but I would expect the occurrences of the third construct (the casting) to be far fewer than those of the first two constructs (where we can determine the type from the context).

In the meantime, our best bet is to just wrap enumerated types in virtual classes to avoid name collisions between literals. I hope you found this post useful. See you next time!

Wednesday, September 3, 2014

Fake It 'til You Make It - Emulating Multiple Inheritance in SystemVerilog

In the last post I talked about interface classes and how they can be used to separate what an object "can" do from "how" it does it. While using interface classes helps a lot in developing code that is modular and reusable, there are still things that are missing. In this discussion on LinkedIn, Ilia makes a very good observation with respect to extendibility. If we extend a base class (in our case, uvm_component), how can we propagate all of these changes to its subclasses (namely uvm_monitor, uvm_driver, etc.)? This is exactly the kind of problem which could be solved with multiple inheritance. Unfortunately, SystemVerilog doesn't support multiple inheritance, so I guess we're out of luck...

Just kidding, as you could probably tell from the title. We can use the tools at our disposal (i.e. single inheritance) to emulate multiple inheritance. The pattern we're going to look at is called a 'mixin'. What better way to illustrate what a mixin is than by looking at an example.

Let's say we want to enforce a rule that all ports of our UVM components are connected before the run phase starts. We create our own super-library based on UVM that contains this addition (and potentially more). We can add such a check to uvm_component by creating a subclass:

class vgm_uvm_component extends uvm_component;
  
  function new(string name, uvm_component parent);
    super.new(name, parent);
  endfunction
  
  function void start_of_simulation_phase(uvm_phase phase);
    check_ports();
  endfunction
  
  function void check_ports();
    `uvm_info("VGM", "I'm checking if all of my ports are connected", UVM_LOW);
  endfunction

endclass

We added the check_ports() function that makes sure that all ports are connected and we called it during the start_of_simulation phase. This will work fine for any class that extends directly from vgm_uvm_component. However, we also want to be able to extend from uvm_driver, uvm_monitor, uvm_agent, etc. These are subclasses of uvm_component, which may not provide too much now, but could potentially do more in future versions of UVM. It also makes our code's intent much clearer. What we need are vgm versions of these classes as well. The simplest way we could do this is by creating one subclass for each, where we copy the additions we made to uvm_component:

class vgm_uvm_monitor extends uvm_monitor;
  
  function new(string name, uvm_component parent);
    super.new(name, parent);
  endfunction
  
  function void start_of_simulation_phase(uvm_phase phase);
    check_ports();
  endfunction
  
  function void check_ports();
    `uvm_info("VGM", "I'm checking if all of my ports are connected", UVM_LOW);
  endfunction
  
endclass

We'd have to do something similar for each uvm_* class, which you'll probably agree is a lot of work. Not only that, but should we find a bug in our code (maybe the check_ports() function isn't working properly), we'd have a lot of places to patch to fix it. This is basically why copy-pasting code is a cardinal sin in the software development world. Sure, we could define the added code as a macro and just include it in every class, but excessive preprocessor use is also something to be avoided.

It sure would be nice if we could make uvm_monitor inherit the changes as well, wouldn't it? It turns out that this is possible. As mentioned above, what we have to do is use a 'mixin'. What we were basically doing above was inheriting from each member of the UVM component class family. Well, why not just make the base class a parameter? Something like:

class vgm_check_ports_mixin #(type T = uvm_component) extends T;
  
  function new(string name, uvm_component parent);
    super.new(name, parent);
  endfunction
  
  function void start_of_simulation_phase(uvm_phase phase);
    check_ports();
  endfunction
  
  function void check_ports();
    `uvm_info("VGM", "I'm checking if all of my ports are connected", UVM_LOW);
  endfunction
  
endclass

We can then create our vgm_* class family by just parameterizing this mixin with the appropriate base class:

typedef vgm_check_ports_mixin #(uvm_component) vgm_uvm_component;
typedef vgm_check_ports_mixin #(uvm_monitor) vgm_uvm_monitor;

We can do this for any UVM component subclass. In our business code, instead of inheriting from uvm_monitor we'll inherit from vgm_uvm_monitor and so on.

I want to take a short break here and give credit where credit is due. The idea isn't mine, as mixins are already used in the software development world. I first heard of them and saw them implemented in SystemVerilog in this thread in the UVM forums. Reading it made me really excited as this technique opens up a whole new world of possibilities.

We've seen how we can use mixins to propagate additions to a base class down along its inheritance hierarchy. Let's now have a look at a slightly different example. In the last post I mentioned how interface classes can be used to write a much cleaner implementation of TLM. The code I showed was:

class uvm_blocking_get_port #(type T=int) implements
  uvm_blocking_get_if #(T);
  // ...
  
  virtual task get(output T t);
    // ...
  endtask
  
  // other uvm_blocking_get_if interface methods ...
endclass


class uvm_nonblocking_get_port #(type T=int) implements
  uvm_nonblocking_get_if #(T);
  // ...
  
  virtual function bit try_get(output T t);
    // ...
  endfunction
  
  // other uvm_nonblocking_get_if interface methods ...
endclass


class uvm_get_port #(type T=int) implements
  uvm_get_if #(T);
  // ...
  
  // uvm_blocking_get_if interface methods ...
  
  // uvm_nonblocking_get_if interface methods ...
endclass

This snippet describes "what" each port can do (by enumerating the methods it has), but side-steps "how" it does it. Some of you may have probably been asking yourselves the following question: "Won't we have to repeat the same implementation of get(...) (for example) in both uvm_blocking_get_port and in uvm_get_port?". The answer is "Yes, we could, but as before, there's a better way". Let's do like in the previous case and start with the "bad" solution and refine it as we go.

A TLM port per definition forwards a method call on it to its connected implementation. This might be another port (which in turn forwards) or an end component which provides the actual implementation. What we first need is some solid ground to stand on. Let's define a base class that handles connecting and holding the implementation:

virtual class tlm_get_port_base #(type IF);
  protected IF m_imp;
  
  function void connect(IF imp);
    m_imp = imp;
  endfunction
endclass

The implementation can be either blocking, non-blocking or both, so we'll leave it as a parameter. Based on this, here's how the blocking get port would look like:

class tlm_blocking_get_port #(type T = int)
  extends tlm_get_port_base #(tlm_blocking_get_if #(T))
  implements tlm_blocking_get_if #(T);
  
  virtual task get(output T t);
    m_imp.get(t);
  endtask
  
  virtual task peek(output T t);
    m_imp.peek(t);
  endtask
  
endclass

It's nothing special, we're just extending the base (parameterized to accept a blocking interface) and defining the implementations of the interface methods. The non-blocking port will look similar:

class tlm_nonblocking_get_port #(type T = int)
  extends tlm_get_port_base #(tlm_nonblocking_get_if #(T))
  implements tlm_nonblocking_get_if #(T);
  
  virtual function bit can_get();
    return m_imp.can_get();
  endfunction
  
  virtual function bit can_peek();
    return m_imp.can_peek();
  endfunction
  
  virtual function bit try_get(output T t);
    if (m_imp.try_get(t))
      return 1;
    return 0;
  endfunction
  
  virtual function bit try_peek(output T t);
    if (m_imp.try_peek(t))
      return 1;
    return 0;
  endfunction
  
endclass

For the "full" get port we have to implement all of these methods. This means copy-pasting the code from both of these classes:

class tlm_get_port #(type T = int)
  extends tlm_get_port_base #(tlm_get_if #(T))
  implements tlm_get_if #(T);
    
  //--------------------------------------------------
  // methods copied from 'tlm_blocking_get_port'
  //--------------------------------------------------
  
  virtual task get(output T t);
    m_imp.get(t);
  endtask
  
  virtual task peek(output T t);
    m_imp.peek(t);
  endtask
  
  
  //--------------------------------------------------
  // methods copied from 'tlm_blocking_get_port'
  //--------------------------------------------------
  
  virtual function bit can_get();
    return m_imp.can_get();
  endfunction
  
  virtual function bit can_peek();
    return m_imp.can_peek();
  endfunction
  
  virtual function bit try_get(output T t);
    if (m_imp.try_peek(t))
      return 1;
    return 0;
  endfunction
  
  virtual function bit try_peek(output T t);
    if (m_imp.can_peek())
      return 1;
    return 0;
  endfunction
  
endclass

This code may not be much (and it will also most likely not change over time), so it might seem harmless. I might even tend to agree in this case, but just think what it would mean if the method definitions were more complicated...

This is another typical case where multiple inheritance would be extremely useful. If we could inherit from both the blocking and the non-blocking ports, we could have our "full" get port without any code duplication. Mixins helped us last time, so let's use them here as well.

Similarly to what we did for the UVM components, we'll implement the additions we make to the tlm_get_port_base class as mixins. First, let's implement the blocking get mixin:

class tlm_blocking_get_mixin #(type T = int, type BASE = tlm_get_port_base #(tlm_blocking_get_if #(T)))
  extends BASE
  implements tlm_blocking_get_if #(T);
  
  virtual task get(output T t);
    m_imp.get(t);
  endtask
  
  virtual task peek(output T t);
    m_imp.peek(t);
  endtask
  
endclass

This is pretty much the blocking get port class, where we've made the base class a parameter. We do the same for the non-blocking port:

class tlm_nonblocking_get_mixin #(type T = int, type BASE = tlm_get_port_base #(tlm_nonblocking_get_if #(T)))
  extends BASE
  implements tlm_nonblocking_get_if #(T);
  
  virtual function bit can_get();
    return m_imp.can_get();
  endfunction
  
  virtual function bit can_peek();
    return m_imp.can_peek();
  endfunction
  
  virtual function bit try_get(output T t);
    if (m_imp.try_get(t))
      return 1;
    return 0;
  endfunction
  
  virtual function bit try_peek(output T t);
    if (m_imp.try_peek(t))
      return 1;
    return 0;
  endfunction
  
endclass

Again, this is pretty much the non-blocking get port class, but with a parameterized base class. Implementing the get ports becomes pretty trivial; we just need to add our mixins to the appropriate base classes:

class tlm_blocking_get_port #(type T=int)
  extends tlm_blocking_get_mixin #(T);
endclass

class tlm_nonblocking_get_port #(type T=int)
  extends tlm_nonblocking_get_mixin #(T);
endclass

class tlm_get_port #(type T=int)
  extends tlm_nonblocking_get_mixin #(T, tlm_blocking_get_mixin #(T, tlm_get_port_base #(tlm_get_if #(T))));
endclass

For the blocking and non-blocking ports we didn't even need to specify the base class, as we used the default values of the parameters. The "full" get port is just a blocking get port with a non-blocking mixin on top (or the other way around; either works). We just have to be careful that the class at the very base is parameterized with the proper implementation type (i.e. tlm_get_if). This is pretty much it. No duplication, less code to maintain and (pretty) easy to understand as well.

Let's summarize what we've learned. Even though SystemVerilog doesn't provide multiple inheritance, using the mixin pattern we can emulate it by using only single inheritance. Mixins can help us propagate extensions down along a class hierarchy (as we did for the UVM component class family). They can also be used to create a class that inherits from two (or even more) parallel sub-classes while avoiding the diamond problem. I for one intend to use more mixins in my code and maybe so should you.

I've added the mixin code, together with some test harnesses and the "bad" code to the blog repository, for those who want to check it out.

See you next time for more SystemVerilog tips and tricks!