windelbouwman / ppci

A compiler for ARM, X86, MSP430, xtensa and more implemented in pure Python
https://ppci.readthedocs.io/en/latest/
BSD 2-Clause "Simplified" License
337 stars 36 forks source link

Implement a simple malloc #114

Closed darleybarreto closed 4 years ago

darleybarreto commented 4 years ago

This PR (related to #113) has some works on the C compiler side to enable heap allocations on *nix systems (depends on brk syscall). Also some additions to the librt/libc parts.

darleybarreto commented 4 years ago

There's some problems with the path of lib.c. What do you think about the organization of the folders?

windelbouwman commented 4 years ago

I think splitting the libc into folders is a very good idea! Still not sure if we should develop a whole new libc, or try to build an existing one like musl, glibc or newlib.

Note that there are more examples making use of the libc, so it might be a good idea to seperate the malloc example from the libc reorganisation.

darleybarreto commented 4 years ago

I thought on these questions a while back. I was thinking on having at most a simple malloc and free functions so we could build on top of it using nothing but python, or just a brk and the allocations would be done by a malloc.py. I will open an issue to expose my ideas soon.

To solve this PR asap, do you recommend to create a lib.c and put the functions the functions that the examples are using?

codecov-commenter commented 4 years ago

Codecov Report

Merging #114 into master will decrease coverage by 0.47%. The diff coverage is 36.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
- Coverage   78.86%   78.39%   -0.48%     
==========================================
  Files         351      351              
  Lines       46938    45926    -1012     
==========================================
- Hits        37019    36004    -1015     
- Misses       9919     9922       +3     
Impacted Files Coverage Δ
ppci/cli/cc.py 75.60% <ø> (ø)
ppci/codegen/irdag.py 95.73% <33.33%> (-2.39%) :arrow_down:
ppci/lang/c/codegenerator.py 95.18% <33.33%> (-0.09%) :arrow_down:
ppci/ir.py 89.98% <50.00%> (-0.50%) :arrow_down:
ppci/lang/llvmir/nodes.py 42.67% <0.00%> (-4.29%) :arrow_down:
ppci/arch/riscv/rvfx_instructions.py 60.60% <0.00%> (-3.88%) :arrow_down:
ppci/arch/riscv/instructions.py 60.97% <0.00%> (-3.02%) :arrow_down:
ppci/arch/riscv/rvf_instructions.py 65.32% <0.00%> (-2.88%) :arrow_down:
ppci/arch/m68k/instructions.py 68.70% <0.00%> (-2.67%) :arrow_down:
ppci/lang/pascal/nodes/types.py 63.75% <0.00%> (-2.34%) :arrow_down:
... and 89 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update aeda64d...bddf653. Read the comment docs.

darleybarreto commented 4 years ago

I am new to these CI/CD tools, what those results mean?

windelbouwman commented 4 years ago

I am new to these CI/CD tools, what those results mean?

Travis runs the python unittests on different versions of python. The codecoverage indicates how much code was covered by the tests and the progression of this by the work in the pull request.

Thanks for your contribution!

pfalcon commented 4 years ago

@windelbouwman: Did you consider merging such PRs as squashed commits (Github should offer such an option even from UI)? Having individual draft commits like:

Adding simple malloc
Adding simple malloc
Restoring some missing declarations
Restoring some missing declarations [2]
Restoring some missing declarations [3]

In the project history is not ideal.

@darleybarreto: Please consider also squashing on your side. Rule of thumb is that if you change some line twice in 2 commits, then it should be 1 commit in the end. You can squash commits together using git rebase -i master in your branch.

windelbouwman commented 4 years ago

@pfalcon , yes, I considered squashing the commits into a single commit, but I was hesitating to do this based on the other pull request that is based upon this work. Now I noticed that the other pull request was rebased, so this work could have been squashed. I agree that this would have a better option in hind sight..