Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RISC-V] Add quirks for riscv to R2RDump #101683

Merged
merged 5 commits into from
May 16, 2024
Merged

[RISC-V] Add quirks for riscv to R2RDump #101683

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
33afb41
[RISC-V] Add quirks for riscv
SzpejnaDawid Apr 29, 2024
68cfb45
[RISC-V] correct code
SzpejnaDawid Apr 30, 2024
cbee233
[RISC-V] correct a type in the comment
SzpejnaDawid May 6, 2024
9227818
correct typo in comment
SzpejnaDawid May 10, 2024
7c96325
[RISC-V] minimize code
SzpejnaDawid May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 183 additions & 0 deletions src/coreclr/tools/r2rdump/CoreDisTools.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,10 @@ public int GetInstruction(RuntimeFunction rtf, int imageOffset, int rtfOffset, o
case Machine.ArmThumb2:
break;

case Machine.RiscV64:
ProbeRiscV64Quirks(rtf, imageOffset, rtfOffset, ref fixedTranslatedLine);
break;

default:
break;
}
Expand Down Expand Up @@ -1209,6 +1213,185 @@ private static bool IsAnotherRuntimeFunctionWithinMethod(int rva, RuntimeFunctio
return false;
}

/// <summary>
/// Improves disassembler output for RiscV64 by adding comments at the end of instructions.
/// </summary>
/// <param name="rtf">Runtime function</param>
/// <param name="imageOffset">Offset within the image byte array</param>
/// <param name="rtfOffset">Offset within the runtime function</param>
/// <param name="instruction">Textual representation of the instruction</param>
private void ProbeRiscV64Quirks(RuntimeFunction rtf, int imageOffset, int rtfOffset, ref string instruction)
{
const int InstructionSize = 4;
uint instr = BitConverter.ToUInt32(_reader.Image, imageOffset + rtfOffset);

if (IsRiscV64JalrInstruction(instr))
{
/*
Supported patterns:
auipc
addi
ld
jalr

auipc
ld
jalr

auipc
addi
ld
ld
jalr

Irrelevant instructions for calle address calculations are skiped.
*/

AnalyzeRiscV64Itype(instr, out uint rd, out uint rs1, out int imm);
uint register = rs1;
int target = imm;

bool isFound = false;
int currentInstrOffset = rtfOffset - InstructionSize;
int currentPC = rtf.StartAddress + currentInstrOffset;
do
{
instr = BitConverter.ToUInt32(_reader.Image, imageOffset + currentInstrOffset);

if (IsRiscV64LdInstruction(instr))
{
AnalyzeRiscV64Itype(instr, out rd, out rs1, out imm);
if (rd == register)
{
target = imm;
register = rs1;
}
}
else if (IsRiscV64AddiInstruction(instr))
{
AnalyzeRiscV64Itype(instr, out rd, out rs1, out imm);
if (rd == register)
{
target =+ imm;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Have you seen a pattern which needs addition for addi instruction? IMO, it doesn't generate two addis for an address calculation. And it doesn't need both immediate values in ld and addi because ld and addi has the same bit size (signed 12 bits) for immediate value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are patterns which use addi. You can find them here, link. As you can see, last three examples show patterns which use addi. Only the first one uses ld to add an immediate value instead of addi

Copy link
Member

Choose a reason for hiding this comment

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

I mean you don't need target += imm. target = imm is enough because address is calculated with imm in auipc and one imm in ld or addi in patterns. If I am wrong, please let me know

Copy link
Contributor Author

@SzpejnaDawid SzpejnaDawid May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with you that target = imm is enough. My explanation for target += imm is that i wanted to cover even those patterns which could exist and i didn't meet them. If you think that we shouldn't worry so much for the future I can change it to target = imm

Copy link
Member

@clamp03 clamp03 May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IMO, we don't need to worry such cases. If there are such cases, it is a bug we need to fix or optimization candidates. :)
If target already has a value when we check addi, it means a value is set from other addi or ld before.

  • We don't need to generate multiple addi for an address calculation.
  • And ld and addi have the same offset bits. So addi is redundant code in case of that target is set by ld before.

However, if you want, you can leave it. That is what I set it a Nit.
Thank you.

register = rs1;
}
}
else if (IsRiscV64AuipcInstruction(instr))
{
AnalyzeRiscV64Utype(instr, out rd, out imm);
if (rd == register)
{
target += currentPC + imm;
isFound = true;
break;
}
}
else
{
// check if callee address is calculated using an unsupported instruction
rd = (instr >> 7) & 0b_11111U;
if (rd == register)
{
break;
}
}

currentInstrOffset -= InstructionSize;
currentPC -= InstructionSize;
} while (currentInstrOffset > 0);

if (isFound)
{
if (!TryGetImportCellName(target, out string targetName) || string.IsNullOrWhiteSpace(targetName))
{
return;
}

instruction = $"{instruction} // {targetName}";
}
}
}

/// <summary>
/// Checks if instruction is auipc.
/// </summary>
/// <param name="instruction">Assembly code of instruction</param>
/// <returns>It returns true if instruction is auipc. Otherwise false</returns>
private bool IsRiscV64AuipcInstruction(uint instruction)
{
const uint OpcodeAuipc = 0b_0010111;
return (instruction & 0x7f) == OpcodeAuipc;
}

/// <summary>
/// Checks if instruction is jalr.
/// </summary>
/// <param name="instruction">Assembly code of instruction</param>
/// <returns>It returns true if instruction is jalr. Otherwise false</returns>
private bool IsRiscV64JalrInstruction(uint instruction)
{
const uint OpcodeJalr = 0b_1100111;
const uint Funct3Jalr = 0b_000;
return (instruction & 0x7f) == OpcodeJalr &&
((instruction >> 12) & 0b_111) == Funct3Jalr;
}

/// <summary>
/// Checks if instruction is addi.
/// </summary>
/// <param name="instruction">Assembly code of instruction</param>
/// <returns>It returns true if instruction is addi. Otherwise false</returns>
private bool IsRiscV64AddiInstruction(uint instruction)
{
const uint OpcodeAddi = 0b_0010011;
const uint Funct3Addi = 0b_000;
return (instruction & 0x7f) == OpcodeAddi &&
((instruction >> 12) & 0b_111) == Funct3Addi;
}

/// <summary>
/// Checks if instruction is ld.
/// </summary>
/// <param name="instruction">Assembly code of instruction</param>
/// <returns>It returns true if instruction is ld. Otherwise false</returns>
private bool IsRiscV64LdInstruction(uint instruction)
{
const uint OpcodeLd = 0b_0000011;
const uint Funct3Ld = 0b_011;
return (instruction & 0x7f) == OpcodeLd &&
((instruction >> 12) & 0b_111) == Funct3Ld;
}

/// <summary>
/// Retrieves output register and immediate value from U-type instruction.
/// </summary>
/// <param name="instruction">Assembly code of instruction</param>
/// <param name="rd">Output register</param>
/// <param name="imm">Immediate value</param>
private void AnalyzeRiscV64Utype(uint instruction, out uint rd, out int imm)
{
// U-type 31 12 11 7 6 0
// [ imm ] [ rd ] [ opcode ]
rd = (instruction >> 7) & 0b_11111U;
imm = unchecked((int)(instruction & (0xfffff << 12)));
}

/// <summary>
/// Retrieves output register, resource register and immediate value from U-type instruction.
/// </summary>
/// <param name="instruction">Assembly code of instruction</param>
/// <param name="rd">Output register</param>
/// <param name="rs1">Resource register</param>
/// <param name="imm">Immediate value</param>
private void AnalyzeRiscV64Itype(uint instruction, out uint rd, out uint rs1, out int imm)
{
// I-type 31 20 19 15 14 12 11 7 6 0
// [ imm ] [ rs1 ] [ funct3 ] [ rd ] [ opcode ]
rd = (instruction >> 7) & 0b_11111U;
rs1 = (instruction >> 15) & 0b_11111U;
imm = unchecked((int)instruction) >> 20;
}

/// <summary>
/// Determine whether a given character is an ASCII digit.
/// </summary>
Expand Down
Loading