Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 24, 2025

  • Explore repository structure and understand existing patterns
  • Create LEA.h in includes/GenieSys/CpuOperations
  • Create LEA.cpp in src/CpuOperations
  • Create LEATest.cpp in test/CpuOperations
  • Include LEA in CpuOperation::getOperations
  • Update src/CMakeLists.txt to include LEA.cpp
  • Update test/CMakeLists.txt to include LEATest.cpp
  • Build and verify all tests pass (763 tests passed)
  • Run code review (no issues found)
  • Clean up build artifacts
  • Add check for invalid EA modes (0b000, 0b001, 0b011, 0b100) that triggers illegal instruction trap
  • Add tests for illegal instruction cases

Fixes #51

Original prompt

This section details on the original issue you should resolve

<issue_title>Implement LEA</issue_title>
<issue_description>Operation: < ea > → An

Assembler Syntax: LEA < ea > ,An

Attributes: Size = (Long)

Description: Loads the effective address into the specified address register. All 32 bits of
the address register are affected by this instruction.

Condition Codes:
Not affected.

Instruction Format: 0100rrr111mmmaaa
r -> An bits
m -> < ea > mode bits
a -> < ea > address bits

Feel free to reference docs/86KPM.pdf for more details about instruction behavior.

Please follow existing patterns in the code. </issue_description>

Comments on the Issue (you are @copilot in this section)


💡 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.

@PaulTrampert
Copy link
Owner

@copilot Please try again.

I would expect the following changes:

  • Add LEA.h to includes/GenieSys/CpuOperations
  • Add LEA.cpp to src/CpuOperations
  • Add LEATest.cpp to test/CpuOperations
  • Include LEA in CpuOperation::getOperations

Co-authored-by: PaulTrampert <5550942+PaulTrampert@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 24, 2025

@copilot Please try again.

I would expect the following changes:...

Done in 7785bee. Added LEA.h, LEA.cpp, LEATest.cpp, and included LEA in CpuOperation::getOperations following the existing patterns in the codebase.

Copilot AI and others added 2 commits November 24, 2025 20:43
Co-authored-by: PaulTrampert <5550942+PaulTrampert@users.noreply.github.com>
Co-authored-by: PaulTrampert <5550942+PaulTrampert@users.noreply.github.com>
@PaulTrampert
Copy link
Owner

@copilot It looks like you specified (A3) as the effective address source and set the value of A3 to 0xAABBCCDD. Rather than copying 0xAABBCCDD into A5, this should copy the 32-bit value found from bus->readLong(0xAABBCCDD) into A5.

Copy link
Contributor Author

Copilot AI commented Nov 24, 2025

@copilot It looks like you specified (A3) as the effective address source and set the value of A3 to 0xAABBCCDD. Rather than copying 0xAABBCCDD into A5, this should copy the 32-bit value found from `b...

I want to clarify the expected behavior. LEA (Load Effective Address) traditionally loads the computed address itself into the destination register, not the data at that address. For LEA (A3),A5, the effective address IS the value in A3 (0xAABBCCDD), so A5 should receive 0xAABBCCDD.

Are you saying that for this codebase, LEA should instead read from memory at the effective address? Or should I use a different addressing mode in the test (like absolute addressing or displacement mode) to make the test more meaningful?

@PaulTrampert
Copy link
Owner

@copilot I'm sorry, your PR is correct. My understanding of the documentation was mistaken.

Copilot AI changed the title [WIP] Add LEA instruction implementation Add LEA instruction implementation Nov 24, 2025
@PaulTrampert
Copy link
Owner

@copilot Please add a check to the instruction that checks for ea modes of 0b000, 0b001, 0b011, and 0b100. If the ea mode is any of these, return cpu->trap(TV_ILLEGAL_INSTR);.

Co-authored-by: PaulTrampert <5550942+PaulTrampert@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 24, 2025

@copilot Please add a check to the instruction that checks for ea modes of 0b000, 0b001, 0b011, and 0b100. If the ea mode is any of these, return cpu->trap(TV_ILLEGAL_INSTR);.

Added the check in 8e2109e. The LEA execute method now returns cpu->trap(TV_ILLEGAL_INSTR) when the EA mode is 0b000 (Dn), 0b001 (An), 0b011 ((An)+), or 0b100 (-(An)).

@PaulTrampert
Copy link
Owner

@copilot Please add a test for the illegal instruction case.

Co-authored-by: PaulTrampert <5550942+PaulTrampert@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 24, 2025

@copilot Please add a test for the illegal instruction case.

Added in 771456b. Added 4 tests covering each of the illegal EA modes (0b000 Data Register Direct, 0b001 Address Register Direct, 0b011 Post-Increment, 0b100 Pre-Decrement), verifying they return 34 cycles and trigger the illegal instruction trap handler.

@PaulTrampert PaulTrampert marked this pull request as ready for review November 24, 2025 22:27
@PaulTrampert PaulTrampert merged commit 70d1470 into main Nov 24, 2025
2 checks passed
@PaulTrampert PaulTrampert deleted the copilot/implement-lea-instruction branch November 24, 2025 22:32
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.

Implement LEA

2 participants