-
Notifications
You must be signed in to change notification settings - Fork 398
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
Add recognition of modern Intel processors to port library and compiler #7350
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -169,9 +169,30 @@ void TR_X86ProcessorInfo::initialize(bool force) | |||||||||||||
case 0x06: | ||||||||||||||
{ | ||||||||||||||
uint32_t extended_model = getCPUModel(_processorSignature) + (getCPUExtendedModel(_processorSignature) << 4); | ||||||||||||||
uint32_t processorStepping = getCPUStepping(_processorSignature); | ||||||||||||||
switch (extended_model) | ||||||||||||||
{ | ||||||||||||||
case 0x55: | ||||||||||||||
case 0xcf: | ||||||||||||||
_processorDescription |= TR_ProcessorIntelEmeraldRapids; break; | ||||||||||||||
case 0x8f: | ||||||||||||||
_processorDescription |= TR_ProcessorIntelSapphireRapids; break; | ||||||||||||||
case 0x6a: // IceLake_X | ||||||||||||||
case 0x6c: // IceLake_D | ||||||||||||||
case 0x7d: // IceLake | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could not find For the client ice lake processors I see
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, see Vol 4., Table 2-1 |
||||||||||||||
case 0x7e: // IceLake_L | ||||||||||||||
_processorDescription |= TR_ProcessorIntelIceLake; break; | ||||||||||||||
case 0x55: // Skylake_X | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how skylake There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both are considered "optimizations" on Skylake. I believe the stepping is what distinguishes them. I didn't include them because I did not find a verifiable stepping value for both, and distinguishing the processor with that much refinement was not my goal (it may be part of your future CPUID refactoring work). Nevertheless I have since found a stepping value for Cascade Lake, and I modified the PR to support it. |
||||||||||||||
if (processorStepping == 7) | ||||||||||||||
{ | ||||||||||||||
_processorDescription |= TR_ProcessorIntelCascadeLake; | ||||||||||||||
} | ||||||||||||||
else | ||||||||||||||
{ | ||||||||||||||
_processorDescription |= TR_ProcessorIntelSkylake; | ||||||||||||||
} | ||||||||||||||
break; | ||||||||||||||
case 0x4e: // Skylake_L | ||||||||||||||
case 0x5e: // Skylake | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whats the meaning behind the suffix in your comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a comment in my latest forced push. I borrowed the suffix conventions for the processors we recognize from the Linux kernel. |
||||||||||||||
_processorDescription |= TR_ProcessorIntelSkylake; break; | ||||||||||||||
case 0x4f: | ||||||||||||||
_processorDescription |= TR_ProcessorIntelBroadwell; break; | ||||||||||||||
|
@@ -270,8 +291,8 @@ OMR::X86::CodeGenerator::initializeX86(TR::Compilation *comp) | |||||||||||||
* | ||||||||||||||
* TODO: Need to figure out from which mode of Broadwell start supporting TM | ||||||||||||||
*/ | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_INTELHASWELL) == getX86ProcessorInfo().isIntelHaswell(), "isIntelHaswell() failed\n"); | ||||||||||||||
if (!comp->target().cpu.is(OMR_PROCESSOR_X86_INTELHASWELL)) | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_INTEL_HASWELL) == getX86ProcessorInfo().isIntelHaswell(), "isIntelHaswell() failed\n"); | ||||||||||||||
if (!comp->target().cpu.is(OMR_PROCESSOR_X86_INTEL_HASWELL)) | ||||||||||||||
{ | ||||||||||||||
if (comp->target().is64Bit()) | ||||||||||||||
{ | ||||||||||||||
|
@@ -303,9 +324,9 @@ OMR::X86::CodeGenerator::initializeX86(TR::Compilation *comp) | |||||||||||||
// Enable software prefetch of the TLH and configure the TLH prefetching | ||||||||||||||
// geometry. | ||||||||||||||
// | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_INTELCORE2) == comp->cg()->getX86ProcessorInfo().isIntelCore2(), "isIntelCore2() failed\n"); | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_INTELNEHALEM) == comp->cg()->getX86ProcessorInfo().isIntelNehalem(), "isIntelNehalem() failed\n"); | ||||||||||||||
if (((!comp->getOption(TR_DisableTLHPrefetch) && (comp->target().cpu.is(OMR_PROCESSOR_X86_INTELCORE2) || comp->target().cpu.is(OMR_PROCESSOR_X86_INTELNEHALEM))) || | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_INTEL_CORE2) == comp->cg()->getX86ProcessorInfo().isIntelCore2(), "isIntelCore2() failed\n"); | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_INTEL_NEHALEM) == comp->cg()->getX86ProcessorInfo().isIntelNehalem(), "isIntelNehalem() failed\n"); | ||||||||||||||
if (((!comp->getOption(TR_DisableTLHPrefetch) && (comp->target().cpu.is(OMR_PROCESSOR_X86_INTEL_CORE2) || comp->target().cpu.is(OMR_PROCESSOR_X86_INTEL_NEHALEM))) || | ||||||||||||||
(comp->getOption(TR_TLHPrefetch) && self()->targetSupportsSoftwarePrefetches()))) | ||||||||||||||
{ | ||||||||||||||
self()->setEnableTLHPrefetching(); | ||||||||||||||
|
@@ -459,9 +480,9 @@ OMR::X86::CodeGenerator::initializeX86(TR::Compilation *comp) | |||||||||||||
// | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.isGenuineIntel() == getX86ProcessorInfo().isGenuineIntel(), "isGenuineIntel() failed\n"); | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.isAuthenticAMD() == getX86ProcessorInfo().isAuthenticAMD(), "isAuthenticAMD() failed\n"); | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_AMDFAMILY15H) == getX86ProcessorInfo().isAMD15h(), "isAMD15h() failed\n"); | ||||||||||||||
TR_ASSERT_FATAL(comp->compileRelocatableCode() || comp->isOutOfProcessCompilation() || comp->compilePortableCode() || comp->target().cpu.is(OMR_PROCESSOR_X86_AMD_FAMILY15H) == getX86ProcessorInfo().isAMD15h(), "isAMD15h() failed\n"); | ||||||||||||||
int32_t boundary; | ||||||||||||||
if (comp->target().cpu.isGenuineIntel() || (comp->target().cpu.isAuthenticAMD() && comp->target().cpu.is(OMR_PROCESSOR_X86_AMDFAMILY15H))) | ||||||||||||||
if (comp->target().cpu.isGenuineIntel() || (comp->target().cpu.isAuthenticAMD() && comp->target().cpu.is(OMR_PROCESSOR_X86_AMD_FAMILY15H))) | ||||||||||||||
boundary = 32; | ||||||||||||||
else | ||||||||||||||
{ | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these asserts really necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, tbh. It has been decades since I looked at this code, but I think there used to be performance considerations for choosing instructions on various processors. But asserts aren't the way to enforce that.
Given what I've come to understand of the CPUID code as part of this PR, it can only be one of those exact processors coming through this path or the assertion will trip. Most of those processors are fairly old, so it seems this path is not taken very frequently (or never at all).(Upon reading the code more closely, I'm not sure this statement is true).Your question is much broader than what this PR is attempting to address. I'd say it be revisited separately.
And if there is value to keeping these asserts for some reason, we should be able to come up with a more efficient way of expressing these conditions. There is a lot of redundant overhead here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue: #7352
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe these asserts were added when, at least in openJ9, we switched to using the portlib for the processor info instead of the
X86ProcessorInfo
class; I guess it was to ensure that the two sources agreed on all queries.I doubt these asserts are still needed.