cambridgehackers / connectal

Connectal is a framework for software-driven hardware development.
MIT License
159 stars 46 forks source link

support the bitwidth calculation of decimal number scoped enumeration #194

Closed GTwhy closed 1 year ago

GTwhy commented 1 year ago

Hi, I need to transfer some scoped enumeration, but I noticed that connectal currently only supports calculating the bitwidth of unscoped enumeration. I added some code to support the bitwidth calculation of decimal number scoped enumeration, which should be sufficient because bsv does not yet support scoped enumeration other than decimal numbers.

For example, the scoped enum type we use is:

typedef enum {
    IBV_QP_STATE               = 1,       // 1 << 0
    IBV_QP_CUR_STATE           = 2,       // 1 << 1
        ···
    IBV_QP_RATE_LIMIT          = 33554432 // 1 << 25
} QpAttrMask deriving(Bits, Eq, FShow);

Modify the typeBitWidth function in cppgen.py:

# from
if item.get('type') == 'Enum':
        return int(math.ceil(math.log(len(item['elements']), 2)))

# to
if item.get('type') == 'Enum':
        enum_width = int(math.ceil(math.log(len(item['elements']), 2)))
        # determines that the element is a scoped enum
        if len(item['elements'][0]) == 2 and item['elements'][0][1] != None:
            # calculate the maximum bit width among all enum items
            enum_width = max([int(math.ceil(math.log(int(element[1]) + 1, 2))) for element in item['elements']])
        return enum_width

But I encountered a type conversion issue, which is a warning and treated as an error. I tried using static_cast<QpAttrMask> to convert the type, but the warning still remains. However, there is no warning when the symbol is = instead of |=.

connectal/bluesim/jni/CntrlRequest.c:70:46: warning: invalid conversion from 'int' to 'QpAttrMask' [-fpermissive]
   70 |         tempdata.host2Cntrl.reqQp.qpAttrMask |= (QpAttrMask)(((tmp>>23)&0x1fful));
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                              |
      |                                              int

Do you have any suggestions for this? If the issue is resolved, I would like to make a pull request. Thank you!

jameyhicks-cmt commented 1 year ago

Please make a PR. Your change works for the case that the method has a single param which is an enum. The failure you are seeing is due to how the generated code reconstructs enum values that cross a word boundary.

Instead of using '|=' it should use '=' and '|': `` tempdata.host2Cntrl.reqQp.qpAttrMask = (QpAttrMask)((int)tempdata.host2Cntrl.reqQp.qpAttrMask | (((tmp>>23)&0x1fful)));

jameyhicks commented 1 year ago

I have a fix for the second problem

jameyhicks commented 1 year ago

Pushed a change to master so your PR should work now

GTwhy commented 1 year ago

Yes, it works. Thank you!

GTwhy commented 1 year ago

Merged so quickly! I really like your project. Thank you again!