exo-lang / exo

Exocompilation for productive programming of hardware accelerators
https://exo-lang.dev
MIT License
307 stars 28 forks source link

update some code in /apps/gemini/src/platform #718

Closed xiuhu17 closed 2 weeks ago

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.07%. Comparing base (3d02a16) to head (54004c2). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #718 +/- ## ======================================= Coverage 88.07% 88.07% ======================================= Files 92 92 Lines 21303 21303 ======================================= Hits 18763 18763 Misses 2540 2540 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

yamaguchi1024 commented 1 month ago

Hi @xiuhu17 ! Thanks for the PR. Could you explain why you had to change the Gemmini malloc code? Was it incorrect in some cases? Could you provide a reproducer?

xiuhu17 commented 1 month ago

Hi @yamaguchi1024 , the reason I change it because the original code looks like:

  uint32_t loc = 0;
  if (i > 0) {
    loc = ACC_BLOCKS[i - 1].loc + ACC_BLOCKS[i - 1].size;
  }
  ACC_BLOCKS[i].size = size;
  ACC_BLOCKS[i].loc = 0;

The uint32_t loc seems to be useless, and the return value will always be 0x80000000.

yamaguchi1024 commented 3 weeks ago

Sorry for dropping the ball on this! The Gemmini malloc files are a bit stale honestly, and I don't think there's any test case that executes this either. I'm really curious how you found this bug. Were you trying to repurpose this code for some other memory allocation?

xiuhu17 commented 2 weeks ago

Sorry for dropping the ball on this! The Gemmini malloc files are a bit stale honestly, and I don't think there's any test case that executes this either. I'm really curious how you found this bug. Were you trying to repurpose this code for some other memory allocation?

Yeh, I previously was trying to figure out the allocation logic used by EXO, I read the code and found that issue. :)

yamaguchi1024 commented 2 weeks ago

@xiuhu17 great, thanks for the contribution!