Eliminate vtr_free and vtr_malloc/vtr_calloc/vtr_realloc

[vaughnbetz]

We have a mix of new/delete and malloc/free in the vpr code base. This is dangerous as someone might free something created with new, or delete something created with malloc.

Proposed Behaviour & Solution

Remove all free/malloc/calloc/realloc calls and replace with new/delete equivalents.

Take some care when dealing with the vtr_chunk_alloc routines, as we still want those efficient memory pooling allocators (if they call new/delete when they actually get the memory they're managing in chunks, that would be sufficient for this clean up).

Holding this for now and will assign to a summer student.

[litghost]

I wonder if all instances of raw malloc/free/new/delete should be using a managed container? There may be a handful of places where the cost std::vector is expensive, but that should be replaced by vtr::light_vector or something.

[kmurray]

I agree using a container to automatically manage memory makes sense.

[github-actions]

This issue has been inactive for a year and has been marked as stale. It will be closed in 15 days if it continues to be stale. If you believe this is still an issue, please add a comment.

[vaughnbetz]

We did most of this. VPR no longer has vtr_malloc or vtr::malloc. Odin II, parmys, libarchfpga and vqm2blif do have vtr::malloc.
It might be worth eliminating vtr::malloc from libarchfpga. @AlexandreSinger @AmirhosseinPoolad : any thoughts? It could be a useful summer student task.

[AlexandreSinger]

I agree with this completely. I am on the side of Kevin on this. Using a container to manage memory is safer and easier to work with. Ideally we should create something similar to LLVM's SmallVector / DenseMap / etc. which have the same interface as std::vector / std::map / etc. but with added memory optimizations for the size of elements expected for compilers.

I agree it would be a useful summer task to have them change all vtr::mallocs into new, and then if they are keen on it potentially abstracting some of them into containers. We can explore creating our own custom containers down the road. I am not sure how much gain that can get, but they were certainly worth it in LLVM!

[vaughnbetz]

Whoops, didn't read all of it. There might be gains from small_vector etc. but since we try not to allocate lots of small vectors I don't think there would be a lot. Keeping this open to get rid of some more vtr::malloc -- I think we should try to get rid of them in libarch_fpga by switching to new.

[AmirhosseinPoolad]

I also agree with this. Changing all malloc/frees to new/delete is definitely doable without spending a lot of time, but the end goal should be using containers everywhere or in case of some super optimized code encapsulating all manual memory management in constructor/destructors (Basically RAII). I'll try to make a list of all places we do malloc/free and also all of the manual memory management.

[vaughnbetz]

Let's use this as a first task for a summer student.

[vaughnbetz]

Task is to remove free/malloc/calloc/realloc from libarchfpga and replace with new and delete.

[SamuelHo10]

What should I do with the vtr::strup function calls? They cannot be deallocated with "delete". Should I replace them with something like this?
char* c_style_str = new char[str.length() + 1]; std::strcpy(c_style_str, str.c_str());
@vaughnbetz

[vaughnbetz]

No, it's probably easier to read a call to vtr::strdup call instead of inline code like that.
I'd replace char* C-strings and strdup with C++ strings (std::string), which will get rid of calls to strdup, and should get rid of explict calls to delete (or free) too.

I'd do that in a separate PR though, so you minimize the odds of a new bug getting in and only being caught by CI (or if it does get in it is easier to find it and fix it).

Also note that we're just trying to clean up libarchfpga, not Odin II, parmys, yosys or abc. Those code bases are primarily owned by other groups, and some of them use strdup a lot so we'll leave them alone.