Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access parent class variable in child class constraint #38

Closed
aneels3 opened this issue Sep 1, 2020 · 15 comments
Closed

Access parent class variable in child class constraint #38

aneels3 opened this issue Sep 1, 2020 · 15 comments

Comments

@aneels3
Copy link
Contributor

aneels3 commented Sep 1, 2020

I was trying to access the parent class variable in child class constraints as in the test example below.

import vsc


@vsc.randobj
class parent:
    def __init__(self):
        self.num = 0
        self.temp = []
    
    def pre_randomize(self):
        for i in range(10):
            self.temp.append(i**2)
        self.num = len(self.temp)
        print("num = ", self.num)


@vsc.randobj
class child(parent):
    def __init__(self):
        super().__init__()
        self.data = vsc.rand_uint32_t()

    @vsc.constraint
    def data_c(self):
        print("Inside data_c")
        print("Inside data_c num = ", self.num)
        self.data < self.num

    def pre_randomize(self):
        super().pre_randomize()

child_ins = child()
print("Before calling randomize()")
child_ins.randomize()
print("After calling randomize()")
print("Data ", child_ins.data)

Output log
image

I found that the constraint of child class being called after the creation of the child class object. Is this expected?
Ideally, the constraint should be checked only after we call the randomize() method.

@mballance
Copy link
Member

Hi @aneels3,
The issue here appears to be that 'num' is not declared as a PyVSC variable. When num is used in a constraint, it's current value is captured. This is all because of how Python implements operator overloading. The constraint failure message (data < 0) reflects this.
I believe this should work if you declare num as a vsc.rand_uint32_t, etc.

Best Regards,
Matthew

@aneels3
Copy link
Contributor Author

aneels3 commented Sep 1, 2020

Hi @mballance
Got it. I have completely missed that. Now It is working as expected.
Thanks!

@aneels3 aneels3 closed this as completed Sep 1, 2020
@aneels3
Copy link
Contributor Author

aneels3 commented Sep 1, 2020

Hi @mballance
I came across a similar problem where I have to use a dict variable of parent class in a child class constraint. As you have suggested above to declare the PyVSC variable in order to use it in a constraint. Do we have something like vsc.rand_list_t() for dictionary variables?

Regards,
Anil

@mballance mballance reopened this Sep 1, 2020
@mballance
Copy link
Member

Hi @aneels3,
We don't currently have a dict that can be used in constraints. Do you happen to have an example that you can share?

Thanks,
Matthew

@aneels3
Copy link
Contributor Author

aneels3 commented Sep 2, 2020

Hi @mballance
please go through the test that I have written for this issue.

import vsc


@vsc.randobj
class parent:
    def __init__(self):
        self.num = vsc.rand_int8_t()
        self.values = {} # This should be of vsc type
        self.name = {
                    0 : {"name":"start", "value":10},
                    1 : {"name":"end", "value":20}
                    }
    def pre_randomize(self):
        if num > 10:
            self.values = self.name

@vsc.randobj
class child(parent):
    def __init__(self):
        super().__init__()
        self.data = vsc.rand_uint32_t()
        self.max_num_t = vsc.rand_uint32_t()

    @vsc.constraint
    def data_c(self):
        self.data < self.num
        with vsc.foreach(self.values, idx=True) as i:
            self.max_num_t = self.values[i].value

    def pre_randomize(self):
        super().pre_randomize()

child_ins = child()
child_ins.randomize()
print("Max number ", child_ins.max_num_t)

@aneels3
Copy link
Contributor Author

aneels3 commented Oct 28, 2020

Hi @mballance
Did you get a chance to look into this issue?

@yashp0103
Copy link

Hi @mballance

As @aneels3 mentioned previously we don't have support for dict type in pyvsc. Can we expect that support?

Thanks,
Yash

@mballance
Copy link
Member

Hi @pvipsyash and @aneels3,
Apologies for the oversight!
@aneels3, the example above appears to be iterating over the elements of a dict and equating the variable to the values (not keys) in the map. Can you help me understand what you're trying to accomplish (ie what your usecase is)? I think there's an issue with the example, and understanding a bit more about what you're trying to do would help me.

Thanks and Best Regards,
Matthew

@aneels3
Copy link
Contributor Author

aneels3 commented Dec 3, 2020

Hi @mballance
Sorry for the minor mistake in the test case that I had provided previously.
Please use the below test case.

import vsc


@vsc.randobj
class parent:
    def __init__(self):
        self.num = vsc.rand_int8_t()
        self.values = [{}] # This should be of vsc type, for eg. list of dict
        self.name = {
                    0 : {"name":"start", "value":10},
                    1 : {"name":"end", "value":20}
                    }
    def pre_randomize(self):
        if num > 10:
            self.values = self.name

@vsc.randobj
class child(parent):
    def __init__(self):
        super().__init__()
        self.data = vsc.rand_uint32_t()
        self.max_num_t = vsc.rand_uint32_t()
        self.name_t = "" # This should be pyvsc str data type 

    @vsc.constraint
    def data_c(self):
        self.data < self.num
        with vsc.foreach(self.values, idx=True) as i:
            self.max_num_t == self.values[i].["value"]
            # Or
            self.name_t == self.values[i].["name"]

    def pre_randomize(self):
        super().pre_randomize()

child_ins = child()
child_ins.randomize()
print("Max number ", child_ins.max_num_t)

Thanks and Regards,
Anil

@mballance
Copy link
Member

Hi @aneels3,
Okay, so it looks like you're looking for a non-rand 'dict-like' type that can be indexed with constraint-foreach index variables? If this is a pattern you're used to using in SystemVerilog, could you share (or point me to) an example?
I see a few potential issues -- especially, for example, if the intent is to have this data type act as a type of constraint. If it's a pretty-simple key/value map for which the primary requirement is that we be able to index it in a foreach, then it's likely easier.

Best Regards,
Matthew

@aneels3
Copy link
Contributor Author

aneels3 commented Dec 8, 2020

Hi @mballance
Please refer to these links for the SV example.
https://github.com/google/riscv-dv/blob/3cdce00e63997201d0f00294365d920ff5e3ba7d/src/riscv_load_store_instr_lib.sv#L64
https://github.com/google/riscv-dv/blob/3cdce00e63997201d0f00294365d920ff5e3ba7d/src/riscv_directed_instr_lib.sv#L48

So in this use-case, the data page is a queue of the associative array.
which is being updated here
https://github.com/google/riscv-dv/blob/3cdce00e63997201d0f00294365d920ff5e3ba7d/src/riscv_directed_instr_lib.sv#L55

To accomplish this using pyvsc I will require a list of dict instead of dict of dict. (Correction)

Regards,
Anil

@mballance
Copy link
Member

Hi @aneels3,
Thanks for sharing these references. I'm starting to get the picture of the current challenges of implementing this approach with pyvsc as it stands today.
I see that 'cfg' sets up some lists of configurations for memory regions. The instruction-stream class then selects one of those lists based on some instruction-stream configuration knobs.
Once difference I note between the SystemVerilog generator and the Python generator is that it appears the instruction-stream classes hold a handle to the config in the SystemVerilog version, while they reference a global object in the Python version. Is this difference desirable (ie it's preferable in Python to use the global object)?
It seems to me that if we could populate the data_page list -- either by changing the handle, or by updating the content of the list -- that this would work with PyVSC. Do you agree? Is there something I'm not understanding about the example?

Thanks and Best Regards,
Matthew

@aneels3
Copy link
Contributor Author

aneels3 commented Dec 14, 2020

Hi @mballance,
Actually, The idea is to create the objects once in the config_gen module and importing the object in different modules throughout the generator in order to minimize multiple object creation of the same class. Even if I create a different handle for cfg that would require a pyvsc list data type which can store the dictionary keys and values of mem_region
mem_region is a dict of dict in our case.

I think I might have made this a little complicated for you.
Let me try to explain it once again.
The self.data_page is being appended with mem_region contents here
which is needed in the addr_c constraint block here.

I want to store the mem_region content to a pyvsc list and use it in the addr_c constraint block based on string and int as key.
Such that self.max_load_store_offset will constraint to corresponding values of size_in_bytes

I hope this makes sense to you.
Please let me know if you still have some doubts about this? or any workaround you can think of!

Thanks and Regards,
Anil

@mballance
Copy link
Member

mballance commented Dec 22, 2020

Hi @aneels3,
After investigating this more, and looking at the related SystemVerilog description, I'd like to suggest an alternate approach. I've created a test illustrating the approach here:

def test_ext_array_1(self):
@vsc.randobj
class region_c(object):
def __init__(self,
name="",
size_in_bytes=-1,
xwr=0):
self.name = name
self.size_in_bytes = vsc.uint32_t(i=size_in_bytes)
self.xwr = xwr = vsc.uint8_t(i=xwr)
@vsc.randobj
class cfg_c(object):
def __init__(self):
self.mem_region = vsc.list_t(region_c())
self.mem_region.extend([
region_c(name="region_0", size_in_bytes=4096, xwr=8),
region_c(name="region_1", size_in_bytes=4096, xwr=8)
])
self.amo_region = vsc.list_t(region_c())
self.amo_region.extend([
region_c(name="amo_0", size_in_bytes=64, xwr=8)
])
self.s_mem_region = vsc.list_t(region_c())
self.s_mem_region.extend([
region_c(name="s_region_0", size_in_bytes=4096, xwr=8),
region_c(name="s_region_1", size_in_bytes=4096, xwr=8)
])
@vsc.randobj
class mem_access_stream_c(object):
def __init__(self, cfg):
self.cfg = cfg
self.max_data_page_id = vsc.int32_t()
self.load_store_shared_memory = False
self.kernel_mode = False
self.data_page = vsc.list_t(region_c())
self.data_page_id = vsc.rand_uint16_t()
self.max_load_store_offset = vsc.rand_uint32_t()
def pre_randomize(self):
self.data_page.clear()
if self.load_store_shared_memory:
self.data_page.extend(self.cfg.amo_region)
elif self.kernel_mode:
self.data_page.extend(self.cfg.s_mem_region)
else:
self.data_page.extend(self.cfg.mem_region)
self.max_data_page_id = len(self.data_page)
@vsc.constraint
def addr_c(self):
self.data_page_id < self.max_data_page_id
with vsc.foreach(self.data_page, idx=True) as i:
with vsc.if_then(i == self.data_page_id):
self.max_load_store_offset == self.data_page[i].size_in_bytes
cfg = cfg_c()
it = mem_access_stream_c(cfg)
it.randomize()
print("len(data_page): " + str(len(it.data_page)))
print("max_data_page_id=" + str(it.max_data_page_id) + " data_page_id=" + str(it.data_page_id))
print("max_load_store_offset=" + str(it.max_load_store_offset))

I'm using a class, region_c, to represent the properties of a memory region. This aligns with the SystemVerilog description. Note that this class is a pyvsc random object, and uses pyvsc fields. I followed the SystemVerilog approach of holding the different types of memory regions within a cfg class. This is an optional element of the example. You can certainly continue to store these at the package level.

The instruction-generation class sets the data_page array within the instruction-generation class to the appropriate array from the configuration.

Note that I did need to correct two things in the library to support this, so you will need to update your pyvsc library.

Does this approach look like it would work?

Thanks and Regards,
Matthew

@aneels3
Copy link
Contributor Author

aneels3 commented Dec 30, 2020

Hi @mballance
Thanks a lot for the WA. It worked perfectly as expected.
Closing it for now!
Regards,
Anil

@aneels3 aneels3 closed this as completed Dec 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants