Merged
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.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes several bugs across different components of the MOSA compiler and kernel, including logical errors in property implementations, incorrect function calls, potential null reference issues, and type casting problems.
Changes:
- Fixed property logic in PageNodeDoubleListEntry that incorrectly checked Next instead of Previous
- Corrected AlignUp method that was incorrectly calling AlignDown
- Added null safety checks in BasePlugTransform and ValueNumberingStage
- Fixed type casting issues in ELF relocation entries where ushort was used instead of ulong
- Corrected spelling of parameter name from "valueVumber" to "valueNumber"
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Source/Mosa.Kernel.BareMetal/PageNodeDoubleListEntry.cs | Fixed IsPreviousPage property to check Previous instead of Next |
| Source/Mosa.Kernel.BareMetal/Alignment.cs | Fixed AlignUp method to call AlignUp instead of AlignDown |
| Source/Mosa.Compiler.Framework/Transforms/Plug/BasePlugTransform.cs | Added null check for context.Operand1 before accessing Method property |
| Source/Mosa.Compiler.Framework/Stages/ValueNumberingStage.cs | Fixed parameter name typo and added null safety checks in IsPhiUseless |
| Source/Mosa.Compiler.Framework/Linker/Elf/RelocationEntry.cs | Fixed Info64 return type from ushort to ulong |
| Source/Mosa.Compiler.Framework/Linker/Elf/RelocationAddendEntry.cs | Fixed Info64 return type from ushort to ulong |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.