Conversation
Renamed the parameter valueVumber to valueNumber in the SetValueNumber method and updated all references accordingly. This improves code clarity and prevents confusion from the previous typo.
Prevent null reference exceptions in IsPhiUseless by checking for null value numbers before comparison. Ensure all operands have valid value numbers before determining if a Phi node is useless.
- Correct Info64 property in relocation entries to use ulong and proper bitwise operations for 64-bit support. - Add null check for context.Operand1 in IsPlugged to prevent exceptions. - Fix IsPreviousPage to check Previous instead of Next for accuracy.
Previously, AlignUp(int, int) incorrectly called AlignDown, causing it to align values downward. This commit corrects the method to call AlignUp((uint), (uint)), ensuring proper upward alignment for integer inputs.
Added a "General Guidelines" section to copilot-instructions.md instructing not to propose or include unsafe code blocks or APIs. Also corrected grammar in the "@Azure Rule - Enable Best Practices" guideline by adding a comma.
- Added explicit guidelines for auto-generated and unit test code in Copilot instructions. - Corrected logic in IfThenElse32.cs Transform method for mixed constant/non-constant operands. - Changed DataRow stub methods to throw NotImplementedException for clearer error reporting.
Changed the Name property from get-only to init-only, allowing it to be set during object initialization while keeping it immutable afterward. This improves flexibility when constructing BaseBlockTransform instances.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR applies a set of small correctness and robustness fixes across the kernel, compiler, and ELF linker code.
Changes:
- Fixes a paging list property (
IsPreviousPage) and an alignment helper (AlignUp(int, int)). - Adjusts x86
IfThenElse32lowering logic and improves plug-matching null-safety. - Cleans up Value Numbering naming/logic and updates ELF relocation “Info64” field type.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Source/Mosa.TinyCoreLib/System.Data/DataRow.cs | Replaces throw null; stubs with NotImplementedException for several APIs. |
| Source/Mosa.Kernel.BareMetal/PageNodeDoubleListEntry.cs | Fixes IsPreviousPage to check Previous instead of Next. |
| Source/Mosa.Kernel.BareMetal/Alignment.cs | Fixes AlignUp(int, int) to call the correct helper. |
| Source/Mosa.Compiler.x86/Transforms/BaseIR/IfThenElse32.cs | Changes conditional-move sequence for a specific constant/non-constant case. |
| Source/Mosa.Compiler.Framework/Transforms/Plug/BasePlugTransform.cs | Adds a null-check for context.Operand1 before dereferencing .Method. |
| Source/Mosa.Compiler.Framework/Stages/ValueNumberingStage.cs | Fixes typo (valueVumber) and tightens IsPhiUseless handling of null VNs. |
| Source/Mosa.Compiler.Framework/Linker/Elf/RelocationEntry.cs | Changes Info64 return type to ulong. |
| Source/Mosa.Compiler.Framework/Linker/Elf/RelocationAddendEntry.cs | Changes Info64 return type to ulong. |
| Source/Mosa.Compiler.Framework/BaseBlockTransform.cs | Changes Name from get-only to init setter. |
| Source/.github/copilot-instructions.md | Adds/clarifies Copilot contribution guidance and fixes punctuation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…2-copilot # Conflicts: # Source/.github/copilot-instructions.md
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Phil Garcia <phil@thinkedge.com>
Corrected Transform method to properly handle cases where only the third operand is a constant. Now initializes the result with the constant and uses a conditional move to select the non-constant operand when appropriate, ensuring correct value selection based on the condition.
…into 522-copilot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.