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

Unify instruction set definition #33730

Merged
merged 6 commits into from
Mar 19, 2020
Merged

Unify instruction set definition #33730

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6952c06
Unify instruction set definition
davidwrighton Mar 18, 2020
4f14632
Fix formatting and build issues
davidwrighton Mar 18, 2020
be186f9
Respond to code review feedback
davidwrighton Mar 18, 2020
26a9810
Fix diffs between old model and new model
davidwrighton Mar 19, 2020
6a65d97
Improve readability of InstructionSetDesc.txt, and fix vm side X86 bu…
davidwrighton Mar 19, 2020
c799dc9
Add 64 bit variant names to 32bit instruction set enum to remove the …
davidwrighton Mar 19, 2020
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
8 changes: 8 additions & 0 deletions src/coreclr/src/inc/corinfoinstructionset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ enum CORINFO_InstructionSet
InstructionSet_POPCNT=15,
InstructionSet_Vector128=16,
InstructionSet_Vector256=17,
InstructionSet_BMI1_X64=18,
Copy link
Member

Choose a reason for hiding this comment

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

Just making sure, do these need to be defined so x86 can report false?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. I got everything working, but I still couldn't build x86 …. and the amount of idefs that were going to be required was entirely unreasonable.

InstructionSet_BMI2_X64=19,
InstructionSet_LZCNT_X64=20,
InstructionSet_POPCNT_X64=21,
InstructionSet_SSE_X64=22,
InstructionSet_SSE2_X64=23,
InstructionSet_SSE41_X64=24,
InstructionSet_SSE42_X64=25,
#endif // TARGET_X86

};
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/src/tools/Common/JitInterface/CorInfoInstructionSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ public enum InstructionSet
X86_POPCNT=15,
X86_Vector128=16,
X86_Vector256=17,
X86_BMI1_X64=18,
X86_BMI2_X64=19,
X86_LZCNT_X64=20,
X86_POPCNT_X64=21,
X86_SSE_X64=22,
X86_SSE2_X64=23,
X86_SSE41_X64=24,
X86_SSE42_X64=25,

}

Expand Down
22 changes: 13 additions & 9 deletions src/coreclr/src/tools/Common/JitInterface/ThunkGenerator/InstructionSetDesc.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

; Definition of X86 instruction sets

definearch ,X86 ,32Bit ,X64
instructionset ,X86 ,Sse , ,1 ,SSE
instructionset ,X86 ,Sse2 , ,2 ,SSE2
implication ,X86 ,SSE2 ,SSE
Expand Down Expand Up @@ -46,18 +47,21 @@ implication ,X86 ,POPCNT ,SSE42
instructionset ,X86 , , , ,Vector128
instructionset ,X86 , , , ,Vector256

; Definition of X64 instruction sets
; Definition of X64 instruction sets (Define )
Copy link
Member

Choose a reason for hiding this comment

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

It seems strange to define instructionset64bit for a non-64bit architecture.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, custom dsls are weird. But avoiding very large diffs that don't really provide value is worth it.

definearch ,X64 ,64Bit ,X64
instructionset64bit,X86 ,BMI1
instructionset64bit,X86 ,BMI2
instructionset64bit,X86 ,LZCNT
instructionset64bit,X86 ,POPCNT
instructionset64bit,X86 ,SSE
instructionset64bit,X86 ,SSE2
instructionset64bit,X86 ,SSE41
instructionset64bit,X86 ,SSE42

copyinstructionsets,X86 ,X64
instructionset64bit,X64 ,BMI1
instructionset64bit,X64 ,BMI2
instructionset64bit,X64 ,LZCNT
instructionset64bit,X64 ,POPCNT
instructionset64bit,X64 ,SSE
instructionset64bit,X64 ,SSE2
instructionset64bit,X64 ,SSE41
instructionset64bit,X64 ,SSE42

; Definition of the Arm64 instruction sets
definearch ,ARM64 ,64Bit ,Arm64
instructionset ,ARM64 ,ArmBase , ,16 ,ArmBase
instructionset64bit,ARM64 ,ArmBase
instructionset ,ARM64 ,AdvSimd , ,17 ,AdvSimd
Expand Down
51 changes: 38 additions & 13 deletions src/coreclr/src/tools/Common/JitInterface/ThunkGenerator/InstructionSetGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public InstructionSetImplication(string architecture, InstructionSetImplication
SortedDictionary<int,string> _r2rNamesByNumber = new SortedDictionary<int,string>();
SortedSet<string> _architectures = new SortedSet<string>();
Dictionary<string,List<string>> _architectureJitNames = new Dictionary<string,List<string>>();
HashSet<string> _64BitArchitectures = new HashSet<string>();
Dictionary<string,string> _64BitVariantArchitectureJitNameSuffix = new Dictionary<string,string>();

void ArchitectureEncountered(string arch)
{
Expand All @@ -92,6 +94,12 @@ void ArchitectureEncountered(string arch)
_architectureJitNames.Add(arch, new List<string>());
}

void ValidateArchitectureEncountered(string arch)
{
if (!_architectures.Contains(arch))
throw new Exception("Architecture not defined");
}

private string ArchToIfDefArch(string arch)
{
if (arch == "X64")
Expand All @@ -102,9 +110,7 @@ private string ArchToIfDefArch(string arch)

private string ArchToInstructionSetSuffixArch(string arch)
{
if (arch == "ARM64")
return "Arm64";
return arch;
return _64BitVariantArchitectureJitNameSuffix[arch];
}

public bool ParseInput(TextReader tr)
Expand All @@ -131,31 +137,45 @@ public bool ParseInput(TextReader tr)
}
switch(command[0])
{
case "definearch":
if (command.Length != 4)
throw new Exception($"Incorrect number of args for definearch {command.Length}");
ArchitectureEncountered(command[1]);
if (command[2] == "64Bit")
{
_64BitArchitectures.Add(command[1]);
}
else if (command[2] != "32Bit")
{
throw new Exception("Architecture must be 32Bit or 64Bit");
}
_64BitVariantArchitectureJitNameSuffix[command[1]] = command[3];
break;
case "instructionset":
if (command.Length != 6)
throw new Exception("Incorrect number of args for instructionset");
ArchitectureEncountered(command[1]);
ValidateArchitectureEncountered(command[1]);
_architectureJitNames[command[1]].Add(command[5]);
_instructionSets.Add(new InstructionSetInfo(command[1],command[2],command[3],command[4],command[5]));
break;
case "instructionset64bit":
if (command.Length != 3)
throw new Exception("Incorrect number of args for instructionset");
ArchitectureEncountered(command[1]);
ValidateArchitectureEncountered(command[1]);
_64bitVariants[command[1]].Add(command[2]);
_architectureJitNames[command[1]].Add(command[2] + "_" + ArchToInstructionSetSuffixArch(command[1]));
break;
case "implication":
if (command.Length != 4)
throw new Exception("Incorrect number of args for instructionset");
ArchitectureEncountered(command[1]);
ValidateArchitectureEncountered(command[1]);
_implications.Add(new InstructionSetImplication(command[1],command[2], command[3]));
break;
case "copyinstructionsets":
if (command.Length != 3)
throw new Exception("Incorrect number of args for instructionset");
ArchitectureEncountered(command[1]);
ArchitectureEncountered(command[2]);
ValidateArchitectureEncountered(command[1]);
ValidateArchitectureEncountered(command[2]);
string arch = command[1];
string targetarch = command[2];
foreach (var val in _instructionSets.ToArray())
Expand All @@ -171,6 +191,11 @@ public bool ParseInput(TextReader tr)
continue;
_implications.Add(new InstructionSetImplication(targetarch, val));
}
foreach (var val in _64bitVariants[arch])
{
_64bitVariants[targetarch].Add(val);
_architectureJitNames[targetarch].Add(val + "_" + ArchToInstructionSetSuffixArch(targetarch));
}
break;
default:
throw new Exception("Unknown command");
Expand Down Expand Up @@ -267,7 +292,7 @@ public static class ReadyToRunInstructionSetHelper
r2rEnumerationValue = $"null";

tr.WriteLine($" case InstructionSet.{architecture}_{instructionSet.JitName}: return {r2rEnumerationValue};");
if (_64bitVariants[architecture].Contains(instructionSet.JitName))
if (_64BitArchitectures.Contains(architecture) && _64bitVariants[architecture].Contains(instructionSet.JitName))
tr.WriteLine($" case InstructionSet.{architecture}_{instructionSet.JitName}_{ArchToInstructionSetSuffixArch(architecture)}: return {r2rEnumerationValue};");
}

Expand Down Expand Up @@ -365,7 +390,7 @@ public static InstructionSetFlags ExpandInstructionSetByImplication(TargetArchit
foreach (var instructionSet in _instructionSets)
{
if (instructionSet.Architecture != architecture) continue;
if (_64bitVariants[architecture].Contains(instructionSet.JitName))
if (_64BitArchitectures.Contains(architecture) && _64bitVariants[architecture].Contains(instructionSet.JitName))
AddImplication(architecture, instructionSet.JitName, $"{instructionSet.JitName}_{ArchToInstructionSetSuffixArch(architecture)}");
}
foreach (var implication in _implications)
Expand Down Expand Up @@ -417,7 +442,7 @@ public void Set64BitInstructionSetVariants(TargetArchitecture architecture)
{
if (instructionSet.Architecture != architecture) continue;

if (_64bitVariants[architecture].Contains(instructionSet.JitName))
if (_64BitArchitectures.Contains(architecture) && _64bitVariants[architecture].Contains(instructionSet.JitName))
{
tr.WriteLine($" if (HasInstructionSet(InstructionSet.{architecture}_{instructionSet.JitName}))");
tr.WriteLine($" AddInstructionSet(InstructionSet.{architecture}_{instructionSet.JitName}_{ArchToInstructionSetSuffixArch(architecture)});");
Expand Down Expand Up @@ -523,7 +548,7 @@ void Set64BitInstructionSetVariants()
{
if (instructionSet.Architecture != architecture) continue;

if (_64bitVariants[architecture].Contains(instructionSet.JitName))
if (_64BitArchitectures.Contains(architecture) && _64bitVariants[architecture].Contains(instructionSet.JitName))
{
tr.WriteLine($" if (HasInstructionSet(InstructionSet_{instructionSet.JitName}))");
tr.WriteLine($" AddInstructionSet(InstructionSet_{instructionSet.JitName}_{ArchToInstructionSetSuffixArch(architecture)});");
Expand Down Expand Up @@ -560,7 +585,7 @@ void SetFromFlagsRaw(uint64_t flags)
foreach (var instructionSet in _instructionSets)
{
if (instructionSet.Architecture != architecture) continue;
if (_64bitVariants[architecture].Contains(instructionSet.JitName))
if (_64BitArchitectures.Contains(architecture) && _64bitVariants[architecture].Contains(instructionSet.JitName))
AddImplication(architecture, instructionSet.JitName, $"{instructionSet.JitName}_{ArchToInstructionSetSuffixArch(architecture)}");
}
foreach (var implication in _implications)
Expand Down