Skip to content

Fix C extension instruction fetch logic and add cross-compilation support#24

Merged
helium729 merged 4 commits intomainfrom
copilot/fix-23
Jun 15, 2025
Merged

Fix C extension instruction fetch logic and add cross-compilation support#24
helium729 merged 4 commits intomainfrom
copilot/fix-23

Conversation

Copy link
Contributor

Copilot AI commented Jun 14, 2025

This PR resolves critical issues with C extension tests and cross-compilation that were causing workflow failures.

Issues Fixed

1. C Extension Test Failures

The C extension tests were failing because the fetch logic didn't properly handle PC alignment when extracting 16-bit compressed instructions from 32-bit memory words.

Problem: When PC incremented by 2 after a compressed instruction, the processor would fetch from the same 32-bit word but incorrectly extract the lower 16 bits instead of the upper 16 bits.

Solution: Enhanced the fetch logic in vigna_core.v to check PC[1] alignment:

  • When PC[1] == 0: Extract lower 16 bits for compressed instructions
  • When PC[1] == 1: Extract upper 16 bits for compressed instructions
// Before: Always used lower 16 bits
if (i_rdata[1:0] != 2'b11) begin
    inst[15:0] <= i_rdata[15:0];  // Always lower 16 bits
end

// After: Check PC alignment  
if (pc[1] == 1'b0) begin
    if (i_rdata[1:0] != 2'b11) begin
        inst[15:0] <= i_rdata[15:0];   // Lower 16 bits when aligned
    end
end else begin
    if (i_rdata[17:16] != 2'b11) begin
        inst[15:0] <= i_rdata[31:16];  // Upper 16 bits when misaligned
    end
end

2. Cross-Compilation Issues

The workflow was skipping cross-compilation because the RISC-V toolchain wasn't available in the CI environment.

Solution: Added installation of gcc-riscv64-linux-gnu package to enable cross-compilation of C test programs.

Test Results

All configuration tests now pass:

  • RV32IC (Base + Compressed): 14/14 tests PASSED ✅
  • RV32IMC (Base + Multiply + Compressed): 14/14 tests PASSED ✅
  • RV32IMC+Zicsr (Full featured): 14/14 tests PASSED ✅
  • Cross-compiled C programs: 13/13 tests PASSED ✅

Additional Improvements

New Makefile Targets

Added comprehensive C extension test infrastructure:

  • c_extension_test: Run C extension testbench
  • c_extension_quick_test: Quick C extension test without waveforms
  • c_extension_syntax: Syntax check for C extension testbench
  • c_extension_wave: View C extension test waveforms

Enhanced Test Coverage

The C extension functionality is now properly tested in multiple configurations:

  • Basic C.LI (load immediate) instructions
  • C.ADDI (add immediate) instructions
  • Mixed 16-bit/32-bit instruction streams
  • PC increment handling for compressed instructions

Before this fix, the comprehensive test suite showed:

Setting up C extension operations test...
  FAIL: C extension result = 20 (expected 15)
Tests Failed: 1

After the fix:

Setting up C extension operations test...  
  PASS: C extension result = 15 (expected 15)
Tests Passed: 14, Tests Failed: 0

Fixes #23.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: helium729 <30749877+helium729@users.noreply.github.com>
Copilot AI changed the title [WIP] Bug fix Fix C extension instruction fetch logic and add cross-compilation support Jun 14, 2025
Copilot AI requested a review from helium729 June 14, 2025 12:54
@helium729 helium729 requested a review from Copilot June 15, 2025 04:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes critical issues with the C extension instruction fetch logic and adds cross-compilation support. Key changes include:

  • Updating instruction fetching in vigna_core.v to properly extract compressed instructions based on PC alignment.
  • Enhancing the C extension testbench to cover both lower and upper 16-bit instruction extraction, including a fallback for misaligned fetches.
  • Adding new Makefile targets for running, compiling, and cleaning up C extension tests and waveforms.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
vigna_core.v Improved fetch logic with PC alignment checks for compressed ops.
sim/c_extension_testbench.v Updated testbench to use packed C extension instructions and refine display messages.
Makefile Added targets and cleanup rules for C extension testing and cross-compilation.
Comments suppressed due to low confidence (1)

Makefile:229

  • Confirm that including the C_EXTENSION files in the cleanup process is intentional and consistent with the handling of other testbench artifacts.
rm -f $(VVP_FILE) $(VCD_FILE) ... $(C_EXTENSION_VVP_FILE) $(C_EXTENSION_VCD_FILE)

helium729 and others added 2 commits June 15, 2025 14:15
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@helium729 helium729 marked this pull request as ready for review June 15, 2025 06:34
@helium729 helium729 merged commit 7387b50 into main Jun 15, 2025
1 check passed
@helium729 helium729 deleted the copilot/fix-23 branch June 16, 2025 03:12
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.

Bug fix

2 participants