Skip to content

Split struct and type definition to allow parameterized types #12

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yanicasa
Copy link

My modification consist in defined macros for the different OBI structures.
In some cases, notably passing type by parameter, the typedef is not useful and it is convenient to have the struct

It should be backward compatible, the old types are not changed but use the new macros

@yanicasa
Copy link
Author

@micprog

@micprog micprog self-requested a review August 21, 2024 14:15
Copy link
Member

@micprog micprog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The update to the typedefs is not consistent, please check the comments and make sure the defines are set in a consistent way.


}
`define OBI_TYPEDEF_ATOP_A_OPTIONAL \
typedef OBI_ATOP_A_OPTIONAL a_optional_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
typedef OBI_ATOP_A_OPTIONAL a_optional_t;
typedef `OBI_ATOP_A_OPTIONAL a_optional_t;

Comment on lines +80 to 83
`define OBI_MINIMAL_R_OPTIONAL(r_optional_t) \
logic r_optional_t;
`define OBI_TYPEDEF_MINIMAL_R_OPTIONAL(r_optional_t) \
typedef logic r_optional_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`define OBI_MINIMAL_R_OPTIONAL(r_optional_t) \
logic r_optional_t;
`define OBI_TYPEDEF_MINIMAL_R_OPTIONAL(r_optional_t) \
typedef logic r_optional_t;
`define OBI_MINIMAL_R_OPTIONAL \
logic
`define OBI_TYPEDEF_MINIMAL_R_OPTIONAL(r_optional_t) \
typedef `OBI_MINIMAL_R_OPTIONAL r_optional_t;

Comment on lines +34 to 37
`define OBI_MINIMAL_A_OPTIONAL(a_optional_t) \
logic a_optional_t;
`define OBI_TYPEDEF_MINIMAL_A_OPTIONAL(a_optional_t) \
typedef logic a_optional_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`define OBI_MINIMAL_A_OPTIONAL(a_optional_t) \
logic a_optional_t;
`define OBI_TYPEDEF_MINIMAL_A_OPTIONAL(a_optional_t) \
typedef logic a_optional_t;
`define OBI_MINIMAL_A_OPTIONAL \
logic
`define OBI_TYPEDEF_MINIMAL_A_OPTIONAL(a_optional_t) \
typedef `OBI_MINIMAL_A_OPTIONAL a_optional_t;

`define OBI_TYPEDEF_MINIMAL_A_OPTIONAL(a_optional_t) \
typedef logic a_optional_t;

`define OBI_TYPEDEF_ATOP_A_OPTIONAL(a_optional_t) \
typedef struct packed { \
`define OBI_ATOP_A_OPTIONAL(a_optional_t) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`define OBI_ATOP_A_OPTIONAL(a_optional_t) \
`define OBI_ATOP_A_OPTIONAL \


`define OBI_DEFAULT_REQ_T(req_t, a_chan_t) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`define OBI_DEFAULT_REQ_T(req_t, a_chan_t) \
`define OBI_DEFAULT_REQ_T(a_chan_t) \

logic req; \
} req_t;

typedef `OBI_DEFAULT_REQ_T(req_t, a_chan_t) req_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
typedef `OBI_DEFAULT_REQ_T(req_t, a_chan_t) req_t;
typedef `OBI_DEFAULT_REQ_T(a_chan_t) req_t;


typedef `OBI_DEFAULT_REQ_T(req_t, a_chan_t) req_t;

`define OBI_REQ_T(req_t, a_chan_t) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`define OBI_REQ_T(req_t, a_chan_t) \
`define OBI_REQ_T(a_chan_t) \

logic rready; \
} req_t;

typedef `OBI_REQ_T(req_t, a_chan_t) req_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
typedef `OBI_REQ_T(req_t, a_chan_t) req_t;
typedef `OBI_REQ_T(a_chan_t) req_t;


typedef `OBI_REQ_T(req_t, a_chan_t) req_t;

`define OBI_RSP_T(rsp_t, r_chan_t) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`define OBI_RSP_T(rsp_t, r_chan_t) \
`define OBI_RSP_T(r_chan_t) \

logic rvalid; \
} rsp_t;

typedef `OBI_RSP_T(rsp_t, r_chan_t) rsp_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
typedef `OBI_RSP_T(rsp_t, r_chan_t) rsp_t;
typedef `OBI_RSP_T(r_chan_t) rsp_t;

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

Successfully merging this pull request may close these issues.

2 participants