selfie icon indicating copy to clipboard operation
selfie copied to clipboard

Collisions between student code and Rotor [cc24]

Open Goubermouche opened this issue 1 year ago • 1 comments

Issue description

When working on the bitwise-shift-execution assignment I've opted (as many will probably do) to declare a func3 section for SLL and SRL instructions (F3_SLL, F3_SRL) - this way we follow the convention that Selfie has been using up until now (there are other F3 declarations as well - ie. F3_SLTU etc. This, however, collides with the declarations of variables in Rotor

The above collision materializes when we run GH actions with the newly pushed code - specifically in the following actions failing due to multiple declarations:

  • Make Everything Selfie / Make all of selfie on Linux (push)
  • Make Everything Selfie / Make everything of selfie on docker (push)

Both of these errors appear to be caused by the inclusion of rotor.c in the compilation stage in the above actions. Note that Autograde Selfie Assignment / Run autograder on linux (push) works just fine.

GH actions failing due to multiple declarations is just an example, the issue is related to the potentially unpredictable connection in the specified actions, another example of a similar issue are functions such as record_lui_addi_add_sub_mul_divu_remu_sltu_jal_jalr - in this case the student may be incentivized to rename the function to something like record_lui_addi_add_sub_mul_divu_remu_sltu_sll_srl_jal_jalr (or rename it altogether) to better match the behavior of the function - this produces errors due to the function also being used in Rotor.

Potential solution

  • Omit the compilation of rotor in student compiler code, ie. by using a specific branch for it. This may not be feasible as I'm not certain how deeply-incorporated rotor is to the 'pure' Selfie project at this moment.
  • Mention this dependency somewhere, again, this is involved as pretty much all assignments only allow the modification of grammar.md, riscu.md, and selfie.c files.

Goubermouche avatar Feb 26 '24 16:02 Goubermouche

Another potential solution is to disable the Make everything selfie workflow in your repository. If I recall correctly, this workflow builds selfie on all operating systems, including macOS.

Github actions running on macOS 'cost' 10 minutes for each minute of runtime. Since the number of available minutes for private repositories is limited, this will also consume your available minutes in a month rather quickly. So you might want to disable the Make everything Selfie workflow either way.

@ckirsch Would this be ok? If yes, I would propose it to all students.

nfejzic avatar Feb 26 '24 17:02 nfejzic

You are welcome to recommend turning off make everything in the channel.

ckirsch avatar Feb 27 '24 07:02 ckirsch

I just pushed an updated Makefile. Feel free to suggest ways to utilize the less target in the channel. I am curious about minimal modifications that are easy to describe.

ckirsch avatar Feb 27 '24 07:02 ckirsch

I just pushed an updated Makefile. Feel free to suggest ways to utilize the less target in the channel. I am curious about minimal modifications that are easy to describe.

I just noticed that macOS action should not run in private repositories:

https://github.com/cksystemsteaching/selfie/blob/7dd64f43ce6013e3b5785687e6138e3682d734ba/.github/workflows/selfie.yml#L62-L65

Should be same for windows. @Goubermouche can you confirm this for me? If I'm right, than this part is fine.

@Goubermouche if that's right, can you then just pull the latest changes from this repository and check if you still have problems?

nfejzic avatar Feb 27 '24 10:02 nfejzic

@nfejzic the Windows target doesn't build for me, as expected (only the Linux one does). The modifications seem to have fixed the issue for me. Thanks a bunch 😄.

Goubermouche avatar Feb 27 '24 14:02 Goubermouche