Fix up the malloc buffers #254
Conversation
|
Would it be possible for this to get merged into main sooner than later? |
| return hash(self._dtype) | ||
| return hash(("MallocBufferFType", self._dtype)) | ||
|
|
||
| def __call__(self, len: int = 0, dtype: type | None = None): |
There was a problem hiding this comment.
IMO dtype is already contained in the MallocBufferFType instance. I think overriding it with an argument will be confusing. Can we drop dtype parameter?
| ctx: "CContext", | ||
| buf: "Stack", | ||
| idx: "AssemblyExpression", | ||
| value: "AssemblyExpression", |
There was a problem hiding this comment.
I don't think we need any of these to be wrapped in "".
| methods: CMallocBufferMethods | None = ctx.datastructures.get(self) | ||
| assert methods is not None, ( | ||
| "A Mallocbuffer must be unpacked before being operated on!" | ||
| ) |
There was a problem hiding this comment.
I prefer if self not in ctx.datastructures: raise Exception
| size_t length | ||
| ) { | ||
| m->length = length; | ||
| m->data = malloc(length * datasize); |
There was a problem hiding this comment.
Do we want if (m->data == NULL) for guards here?
| ] | ||
|
|
||
|
|
||
| class CMallocBufferMethods(TypedDict): |
There was a problem hiding this comment.
Can we make it just a dataclass and access elements with methods.init etc?
| t = ctx.ctype_name(c_type(self.element_type)) | ||
| ctx.add_header("#include <stddef.h>") | ||
|
|
||
| ctx.add_datastructure( |
There was a problem hiding this comment.
Can we simplify it and get rid of def add_datastructure? Instead:
if self not in ctx.datastructures:
MallocBufferBackend.gen_code(ctx, self, inline=True)| ctx: "CContext", | ||
| ftype: "MallocBufferFType", | ||
| inline: bool = False, | ||
| ): |
| if ftype in cls._library: | ||
| return cls._library[ftype] | ||
| ctx = CContext() | ||
| methods = cls.gen_code(ctx, ftype) |
There was a problem hiding this comment.
I would prefer to arrange it a bit differently.
You call gen_code with inline=True in c_unpack method. In library you call without it. I assume that def library uses it to fetch the methods for MallocBuffer.__init__, MallocBuffer.__del__ and MallocBuffer.resize. I assume that we want to call gen_code for code generation only once in known compilation phase, and other calls are only for fetching methods, right?
Can we decide where/when gen_code happens exactly? and call gen_code only there? In c_unpack?
Right now in some time in the future calling init, resize, and free will cause gen_code to be called three times.
If you have enough time, could you add one test which calls MallocBuffer.__init__, __del__, and resize?
Is there a test which calls MallocBufferFType.c_unpack?
Adding fixes to malloc buffers so they are compliant with the rest of the backend.