[compiler-rt][X86] Unify getIntelProcessorTypeAndSubtype #97861
Conversation
This patch unifies getIntelProcessorTypeAndSubtype between LLVM and compiler-rt as there is a reasonable amount of divergence between the two implementations. This patch is one step in refactoring out several blocks of code in these two files to identical .inc files that can be tested that they are the same to better facilitate code sharing between LLVM and compiler-rt.
boomanaiden154
requested review from
MaskRay,
RKSimon,
phoebewang,
KanRobert and
FreddyLeaf
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| @@ -512,15 +523,15 @@ static const char *getIntelProcessorTypeAndSubtype(unsigned Family, | |||
| // Alderlake: | |||
| case 0x97: | |||
| case 0x9a: | |||
| // Gracemont: | |||
There was a problem hiding this comment.
That's how it was on the LLVM side, and I figured moving it into alphabetical order by codename made more sense than sticking it at the end.
No real preference here though. The only thing I'm trying to get is consistency between the two implementations.
There was a problem hiding this comment.
@FreddyLeaf where's the Gracemont model from? I didn't find it in ISE. Besides, the Gracemont is not CPU name but microarchitecture name.
|
Bump on this when reviewers have a chance. Thanks! |
| @@ -646,10 +657,102 @@ static const char *getIntelProcessorTypeAndSubtype(unsigned Family, | |||
| *Type = INTEL_KNM; | |||
| break; | |||
|
|
|||
| default: // Unknown family 6 CPU. | |||
| default: // Unknown family 6 CPU, try to guess. | |||
There was a problem hiding this comment.
I recall I saw discussion somewhere says it's not proved returning a guessed CPU is better than unknown one.
There was a problem hiding this comment.
Do you have a link? Should we remove this then? This just unifies the implementations, so if we shouldn't guess, then we should remove it from the LLVM implementation.
More than happy to put up a patch for that if that's what is desired.
There was a problem hiding this comment.
No, I can't find it. I'm even not quite sure of it. I think we can put both way in an RFC and see others' opinion. Do you know the behavior of libgcc? It would be a good reference.
This patch unifies getIntelProcessorTypeAndSubtype between LLVM and compiler-rt as there is a reasonable amount of divergence between the two implementations.
This patch is one step in refactoring out several blocks of code in these two files to identical .inc files that can be tested that they are the same to better facilitate code sharing between LLVM and compiler-rt.