[PATCH] D100085: [X86] Support -march=rocketlake

Freddy, Ye via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 7 22:12:46 PDT 2021


FreddyYe added a comment.

THX for review!



================
Comment at: clang/test/Preprocessor/predefined-arch-macros.c:1396
 
+// RUN: %clang -march=rocketlake -m32 -E -dM %s -o - 2>&1 \
+// RUN:     -target i386-unknown-linux \
----------------
MaskRay wrote:
> The file may need some refactoring first. You can let RUN lines share some common check prefixes, instead of adding a bunch of defines for every new processor.
> 
> ```
> // CHECK_X86_64_V2: ...
> // CHECK_X86_64_V2: ...
> // CHECK_X86_64_V3: ...
> // CHECK_PROCESSOR1_M32:
> // CHECK_PROCESSOR1_M64:
> // CHECK_PROCESSOR2_M32:
> // CHECK_PROCESSOR2_M64:
> ```
I agree. I'll do it


================
Comment at: compiler-rt/lib/builtins/cpu_model.c:101
   INTEL_COREI7_ALDERLAKE,
+  INTEL_COREI7_ROCKETLAKE,
   AMDFAM19H_ZNVER3,
----------------
craig.topper wrote:
> This order is defined by libgcc. We can't insert in the middle unless ZNVER3 was in the wrong place
> 
> Why this not referenced in the switch the select subtype?
This is a mistake. I'll modify. And reference is missing in two switch. I'll add.


================
Comment at: llvm/lib/Target/X86/X86.td:767
+  // Rocketlake
+  list<SubtargetFeature> RKLAdditionalFeatures = [FeatureAES,
+                                                  FeatureXSAVEC,
----------------
craig.topper wrote:
> Is this list this long because SKL includes SGX but RKL doesn't?
Yes. And I don't know any simple ways to exclude SGX here, any suggestions?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100085/new/

https://reviews.llvm.org/D100085



More information about the cfe-commits mailing list