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.

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

[RISC-V] Add crossgen2 for riscv64 #95188

Merged
merged 3 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions src/coreclr/tools/Common/CommandLineHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public static TargetArchitecture GetTargetArchitecture(string token)
Architecture.Arm => TargetArchitecture.ARM,
Architecture.Arm64 => TargetArchitecture.ARM64,
Architecture.LoongArch64 => TargetArchitecture.LoongArch64,
(Architecture)9 => TargetArchitecture.RiscV64, /* TODO: update with Architecture.RiscV64 */
Copy link
Member

Choose a reason for hiding this comment

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

Is there a tracking work item for this TODO? It would be for the best to keep hard coded numbers like this one for as short as possible.

_ => throw new NotImplementedException()
};
}
Expand All @@ -108,6 +109,7 @@ public static TargetArchitecture GetTargetArchitecture(string token)
"arm" or "armel" => TargetArchitecture.ARM,
"arm64" => TargetArchitecture.ARM64,
"loongarch64" => TargetArchitecture.LoongArch64,
"riscv64" => TargetArchitecture.RiscV64,
_ => throw new CommandLineException($"Target architecture '{token}' is not supported")
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly)
loongarch64Emitter.Builder.AddSymbol(this);
return loongarch64Emitter.Builder.ToObjectData();

case TargetArchitecture.RiscV64:
RiscV64.RiscV64Emitter riscv64Emitter = new RiscV64.RiscV64Emitter(factory, relocsOnly);
EmitCode(factory, ref riscv64Emitter, relocsOnly);
riscv64Emitter.Builder.RequireInitialAlignment(alignment);
riscv64Emitter.Builder.AddSymbol(this);
return riscv64Emitter.Builder.ToObjectData();

default:
throw new NotImplementedException();
}
Expand All @@ -85,5 +92,6 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly)
protected abstract void EmitCode(NodeFactory factory, ref ARM.ARMEmitter instructionEncoder, bool relocsOnly);
protected abstract void EmitCode(NodeFactory factory, ref ARM64.ARM64Emitter instructionEncoder, bool relocsOnly);
protected abstract void EmitCode(NodeFactory factory, ref LoongArch64.LoongArch64Emitter instructionEncoder, bool relocsOnly);
protected abstract void EmitCode(NodeFactory factory, ref RiscV64.RiscV64Emitter instructionEncoder, bool relocsOnly);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,9 @@ public void EmitReloc(ISymbolNode symbol, RelocType relocType, int delta = 0)

case RelocType.IMAGE_REL_BASED_LOONGARCH64_PC:
case RelocType.IMAGE_REL_BASED_LOONGARCH64_JIR:

//TODO: consider removal of IMAGE_REL_BASED_RISCV64_JIR from runtime too
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update IMAGE_REL_BASED_RISCV64_JIR to IMAGE_REL_RISCV64_JALR.
In the runtime, IMAGE_REL_RISCV64_JALR is used.

case RelocType.IMAGE_REL_BASED_RISCV64_PC:
Debug.Assert(delta == 0);
// Do not vacate space for this kind of relocation, because
// the space is embedded in the instruction.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public enum RelocType
IMAGE_REL_BASED_ARM64_BRANCH26 = 0x15, // Arm64: B, BL
IMAGE_REL_BASED_LOONGARCH64_PC = 0x16, // LoongArch64: pcaddu12i+imm12
IMAGE_REL_BASED_LOONGARCH64_JIR = 0x17, // LoongArch64: pcaddu18i+jirl
IMAGE_REL_BASED_RISCV64_PC = 0x18, // RiscV64: auipc
IMAGE_REL_BASED_RELPTR32 = 0x7C, // 32-bit relative address from byte starting reloc
// This is a special NGEN-specific relocation type
// for relative pointer (used to make NGen relocation
Expand Down Expand Up @@ -411,6 +412,47 @@ private static unsafe void PutLoongArch64JIR(uint* pCode, long imm38)
Debug.Assert(GetLoongArch64JIR(pCode) == imm38);
}

private static unsafe int GetRiscV64PC12(uint* pCode)
Copy link
Contributor

Choose a reason for hiding this comment

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

As I know, PC12 means pcaddu12i in LOONGARCH64. Please update the name to GetRiscV64AUIPC or GetRiscV64PC, Please remove 12 at least.

Suggested change
private static unsafe int GetRiscV64PC12(uint* pCode)
private static unsafe int GetRiscV64PC(uint* pCode)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I’ll fix this

{
uint auipcInstr = *pCode;
Debug.Assert((auipcInstr & 0x7f) == 0x00000017);
// first get the high 20 bits,
int imm = (int)((auipcInstr & 0xfffff000));
// then get the low 12 bits,
uint addiInstr = *(pCode + 1);
Debug.Assert((addiInstr & 0x707f) == 0x00000013);
imm += ((int)(addiInstr)) >> 20;

return imm;
}

// INS_OPTS_RELOC: placeholders. 2-ins:
// case:EA_HANDLE_CNS_RELOC
// auipc reg, off-hi-20bits
// addi reg, reg, off-lo-12bits
// case:EA_PTR_DSP_RELOC
// auipc reg, off-hi-20bits
// ld reg, reg, off-lo-12bits
private static unsafe void PutRiscV64PC12(uint* pCode, long imm32)
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the name too.

Suggested change
private static unsafe void PutRiscV64PC12(uint* pCode, long imm32)
private static unsafe void PutRiscV64PC(uint* pCode, long imm32)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I’ll fix this

{
// Verify that we got a valid offset
Debug.Assert((int)imm32 == imm32);

int doff = (int)(imm32 & 0xfff);
uint auipcInstr = *pCode;
Debug.Assert((auipcInstr & 0x7f) == 0x00000017);

auipcInstr |= (uint)((imm32 + 0x800) & 0xfffff000);
*pCode = auipcInstr;

uint addiInstr = *(pCode + 1);
Debug.Assert((addiInstr & 0x707f) == 0x00000013);
addiInstr |= (uint)((doff & 0xfff) << 20);
*(pCode + 1) = addiInstr;

Debug.Assert(GetRiscV64PC12(pCode) == imm32);
}

public Relocation(RelocType relocType, int offset, ISymbolNode target)
{
RelocType = relocType;
Expand Down Expand Up @@ -455,6 +497,9 @@ public static unsafe void WriteValue(RelocType relocType, void* location, long v
case RelocType.IMAGE_REL_BASED_LOONGARCH64_JIR:
PutLoongArch64JIR((uint*)location, value);
break;
case RelocType.IMAGE_REL_BASED_RISCV64_PC:
PutRiscV64PC12((uint*)location, value);
break;
default:
Debug.Fail("Invalid RelocType: " + relocType);
break;
Expand Down Expand Up @@ -517,6 +562,8 @@ public static unsafe long ReadValue(RelocType relocType, void* location)
return (long)GetLoongArch64PC12((uint*)location);
case RelocType.IMAGE_REL_BASED_LOONGARCH64_JIR:
return (long)GetLoongArch64JIR((uint*)location);
case RelocType.IMAGE_REL_BASED_RISCV64_PC:
return (long)GetRiscV64PC12((uint*)location);
default:
Debug.Fail("Invalid RelocType: " + relocType);
return 0;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace ILCompiler.DependencyAnalysis.RiscV64
{
public enum AddrModeSize
{
Int8 = 1,
Int16 = 2,
Int32 = 4,
Int64 = 8,
Int128 = 16
}

public struct AddrMode
{
public readonly Register BaseReg;
public readonly Register? IndexReg;
public readonly int Offset;
public readonly byte Scale;
public readonly AddrModeSize Size;

public AddrMode(Register baseRegister, Register? indexRegister, int offset, byte scale, AddrModeSize size)
{
BaseReg = baseRegister;
IndexReg = indexRegister;
Offset = offset;
Scale = scale;
Size = size;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace ILCompiler.DependencyAnalysis.RiscV64
{
public enum Register
{
X0,
X1,
X2,
X3,
X4,
X5,
X6,
X7,
X8,
X9,
X10,
X11,
X12,
X13,
X14,
X15,
X16,
X17,
X18,
X19,
X20,
X21,
X22,
X23,
X24,
X25,
X26,
X27,
X28,
X29,
X30,
X31,

None,
NoIndex = 128
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;

namespace ILCompiler.DependencyAnalysis.RiscV64
{
public struct RiscV64Emitter
{
public RiscV64Emitter(NodeFactory factory, bool relocsOnly)
{
Builder = new ObjectDataBuilder(factory, relocsOnly);
TargetRegister = new TargetRegisterMap(factory.Target.OperatingSystem);
}

public ObjectDataBuilder Builder;
public TargetRegisterMap TargetRegister;

// Assembly stub creation api. TBD, actually make this general purpose

//ebreak
public void EmitBreak()
{
Builder.EmitUInt(0x00100073);
}

public void EmitLI(Register regDst, ushort imm12)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add assert for imm12.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I’ll fix this

EmitADDI(regDst, Register.X0, imm12);
}

public void EmitMOV(Register regDst, Register regSrc)
{
EmitADDI(regDst, regSrc, 0);
}

public void EmitMOV(Register regDst, ISymbolNode symbol)
{
Builder.EmitReloc(symbol, RelocType.IMAGE_REL_BASED_RISCV64_PC);
//auipc reg, off-hi-20bits
EmitPC(regDst);
//addi reg, reg, off-lo-12bits
EmitADDI(regDst, regDst, 0);
}

// auipc regDst, 0
public void EmitPC(Register regDst)
{
Debug.Assert((uint)regDst > 0 && (uint)regDst < 32);
Builder.EmitUInt(0x00000017u | (uint)regDst << 7);
}

// addi regDst, regSrc, offset
public void EmitADDI(Register regDst, Register regSrc, int offset)
{
Debug.Assert((uint)regDst <= 0x1f);
Debug.Assert((uint)regSrc <= 0x1f);
Debug.Assert((offset >= -2048) && (offset <= 2047));
Builder.EmitUInt((uint)(0x00000013u | ((uint)regSrc << 15) | ((uint)regDst << 7) | (uint)((offset & 0xfff) << 20)));
}

// xori regDst, regSrc, offset
public void EmitXORI(Register regDst, Register regSrc, int offset)
{
Debug.Assert((offset >= -2048) && (offset <= 2047));
Builder.EmitUInt((uint)(0x00004013u | ((uint)regSrc << 15) | ((uint)regDst << 7) | (uint)((offset & 0xfff) << 20)));
}

// ld regDst, offset(regSrc)
public void EmitLD(Register regDst, Register regSrc, int offset)
{
Debug.Assert((offset >= -2048) && (offset <= 2047));
Builder.EmitUInt((uint)(0x00003003u | ((uint)regSrc << 15) | ((uint)regDst << 7) | (uint)((offset & 0xfff) << 20)));
}

// jalr regDst, offset(regSrc)
public void EmitJALR(Register regDst, Register regSrc, int offset)
{
Debug.Assert((offset >= -2048) && (offset <= 2047));
Builder.EmitUInt((uint)(0x00000067u | ((uint)regSrc << 15) | ((uint)regDst << 7) | (uint)((offset & 0xfff) << 20)));
}

public void EmitRET()
{
// jalr x0,0(x1)
EmitJALR(Register.X0, Register.X1, 0);
}

public void EmitJMP(Register reg)
{
//jalr x0, 0(reg)
EmitJALR(Register.X0, reg, 0);
}

public void EmitJMP(ISymbolNode symbol)
{
if (symbol.RepresentsIndirectionCell)
{
//auipc x29, 0
EmitPC(Register.X29);
//ld x29,16(x29)
EmitLD(Register.X29, Register.X29, 16);
//ld x29,0(x29)
EmitLD(Register.X29, Register.X29, 0);
//jalr x0,0(x29)
EmitJALR(Register.X0, Register.X29, 0);

Builder.EmitReloc(symbol, RelocType.IMAGE_REL_BASED_DIR64);
}
else
{
Builder.EmitUInt(0x00000000); // bad code.
throw new NotImplementedException();
}
}

public void EmitRETIfZero(Register regSrc)
{
// bne regSrc, x0, 8
Builder.EmitUInt((uint)(0x00001463 | ((uint)regSrc << 15)));
EmitRET();
}

public void EmitJE(Register regSrc, ISymbolNode symbol)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think EmitJNEZ is better.

Copy link
Member

Choose a reason for hiding this comment

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

It's actually the same as for other arches meaning that we jump over the stuff generated by emitjmp in case condition is reversed (in this case ne)

Copy link
Member

@clamp03 clamp03 Dec 22, 2023

Choose a reason for hiding this comment

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

I think EmitJE naming is for ARM* arch which use flag after cmp instruction. However, in RISC-V it doesn't compare and jump like ARM*. So I thought it is better if it shows RISC-V's code generation like what you changed EmitRetIfEqual to EmitRetIfZero.
I am confusing. I think EmitJEZ seems right.

This is just my suggestion. You can freely choose. :)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, my bad, thought about the other thing, yes, you are right :) I meant that it's not EmitJNotfZero, but EmitJIfZero

{
uint offset = symbol.RepresentsIndirectionCell ? 14u : 4u;
uint encodedOffset = ((offset & 0x1e) << 7) | ((offset & 0x7e0) << 20) | ((offset & 0x800) >> 4) | ((offset & 0x1000) << 19);
Copy link
Contributor

Choose a reason for hiding this comment

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

With your encodedOffset logic, offset should be 7 << 2 : 2 << 2. Please check again.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

// bne regSrc, x0, offset
Builder.EmitUInt((uint)(0x00001063 | ((uint)regSrc << 15) | encodedOffset));
EmitJMP(symbol);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Internal.TypeSystem;

namespace ILCompiler.DependencyAnalysis.RiscV64
{
/// <summary>
/// Maps logical registers to physical registers on a specified OS.
/// </summary>
public struct TargetRegisterMap
{
public readonly Register Arg0;
public readonly Register Arg1;
public readonly Register Arg2;
public readonly Register Arg3;
public readonly Register Arg4;
public readonly Register Arg5;
public readonly Register Arg6;
public readonly Register Arg7;
public readonly Register IntraProcedureCallScratch1;
public readonly Register Result;

public TargetRegisterMap(TargetOS os)
{
Arg0 = Register.X10;
Arg1 = Register.X11;
Arg2 = Register.X12;
Arg3 = Register.X13;
Arg4 = Register.X14;
Arg5 = Register.X15;
Arg6 = Register.X16;
Arg7 = Register.X17;
IntraProcedureCallScratch1 = Register.X28;
Result = Register.X10;
}
}
}
4 changes: 4 additions & 0 deletions src/coreclr/tools/Common/Compiler/InstructionSetSupport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ public SimdVectorLength GetVectorTSimdVector()
{
return SimdVectorLength.None;
}
else if (_targetArchitecture == TargetArchitecture.RiscV64)
{
return SimdVectorLength.None;
}
else
{
Debug.Assert(false); // Unknown architecture
Expand Down
Loading
Loading