Skip to content

Conversation

hhhh1300
Copy link
Contributor

@hhhh1300 hhhh1300 commented Sep 29, 2025

Add short data type support

This PR implements support for the short data type (#282).

Changes Made

  • Parser & Type System: Extended parser with TYPE_short cases for all operations
  • ARM/RISC-V Architecture: Added halfword instructions.
  • Test Coverage: Expanded test suite with comprehensive short type validation

Testing

  • All tests pass on both Stage 0 and Stage 2

Summary by cubic

Add full support for the short (2-byte) type across the parser, type system, and ARM/RISC-V codegen, with correct halfword loads/stores, truncation, and sign extension. Expanded tests cover variables, pointers, arrays, casts, and sizeof.

  • New Features
    • Parser/type system: added TYPE_short and TY_short (2 bytes); handled dereference, lvalues, compound literals, pointer arithmetic, and sizeof.
    • ARM: implemented halfword transfers (__lh LDRSH, __sh), SXTH sign extension, fixed OP_trunc via shifts, updated OP_read/write and ELF offsets.
    • RISC-V: added lh/sh operations, fixed OP_trunc using shift pairs, implemented sign extension for byte/short, updated ELF offsets.
    • Tests: added short cases for assignment and arithmetic, pointers and deref, arrays, compound literals, function params/returns, casts, and sizeof.

@jserv jserv requested review from ChAoSUnItY and DrXiao and removed request for ChAoSUnItY September 29, 2025 13:46
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="tests/driver.sh">

<violation number="1" location="tests/driver.sh:5061">
The new cast_tests array is never executed because there is no run_* invocation for it, so these added tests never run. Please add the corresponding runner call so the short casts are actually tested.</violation>
</file>


Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Ask questions if you need clarification on any suggestion

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@jserv jserv requested a review from visitorckw September 29, 2025 14:52
@jserv
Copy link
Collaborator

jserv commented Sep 29, 2025

Once you are confident that you have addressed the reviewers’ suggestions, simply click the "Resolve conversation" button in the GitHub web interface.

Copy link
Collaborator

@ChAoSUnItY ChAoSUnItY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you missed to change logic in reg-alloc.c as well, some stack alignment are done there.

Hint: look at OP_allocat usages in reg-alloc.c, you should also consider TY_short cases there.

Copy link
Collaborator

@DrXiao DrXiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the proposed changes enable shecc to parse short int ?

If so, add some test cases to validate.

@hhhh1300
Copy link
Contributor Author

This PR doesn't enable short int. I think this may require further changes on parser.c.

It causes "Unexpected token" error when parsing short int.

@sysprog21 sysprog21 deleted a comment from cubic-dev-ai bot Sep 30, 2025
@sysprog21 sysprog21 deleted a comment from cubic-dev-ai bot Sep 30, 2025
cubic-dev-ai[bot]

This comment was marked as outdated.

cubic-dev-ai[bot]

This comment was marked as outdated.

Copy link
Collaborator

@visitorckw visitorckw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase your branch to keep the git history clean.

@hhhh1300 hhhh1300 force-pushed the feature/complete-short-type-support branch from f381006 to 0a327fa Compare October 1, 2025 04:00
Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check https://cbea.ms/git-commit/ carefully and enforce the rules.

@hhhh1300 hhhh1300 force-pushed the feature/complete-short-type-support branch from 0a327fa to c0e08ca Compare October 1, 2025 06:16
@hhhh1300 hhhh1300 force-pushed the feature/complete-short-type-support branch from c0e08ca to 191e76f Compare October 1, 2025 14:16
Previously, shecc only supported int, char, void, and _Bool as basic
types. Adding short type enhances C99 compliance and enables proper
handling of 16-bit signed integers for memory-efficient data structures.

This implementation extends the type system with TY_short, adds halfword
load/store instructions to both backends, and fixes OP_sign_ext to encode
both source and target type sizes for correct sign extension behavior.
@hhhh1300 hhhh1300 force-pushed the feature/complete-short-type-support branch from 191e76f to 5cb9b03 Compare October 1, 2025 14:24
Copy link
Collaborator

@DrXiao DrXiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these changes look good.

@jserv jserv merged commit c4f778e into sysprog21:master Oct 1, 2025
6 checks passed
@jserv
Copy link
Collaborator

jserv commented Oct 1, 2025

Thank @hhhh1300 for contributing!

@hhhh1300 hhhh1300 deleted the feature/complete-short-type-support branch October 2, 2025 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants