[PATCH] D78129: Add Marvell ThunderX3T110 support

[Wei Zhao via Phabricator via cfe-commits]

wxz2020 marked 7 inline comments as done.
wxz2020 added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:89
     CPU = llvm::sys::getHostCPUName();
-
   if (CPU == "generic") {
----------------
DavidSpickett wrote:
> Remove stray change.
Removed


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:167
 AARCH64_CPU_NAME("thunderx2t99", ARMV8_1A, FK_CRYPTO_NEON_FP_ARMV8, false,
-                 (AArch64::AEK_NONE))
+                 (AArch64::AEK_CRC | AEK_CRYPTO | AEK_FP | AEK_SIMD | AEK_LSE | AEK_RAND))
+AARCH64_CPU_NAME("thunderx3t110", ARMV8_3A, FK_CRYPTO_NEON_FP_ARMV8, false,
----------------
DavidSpickett wrote:
> Is this change also correcting the options for the x2t99? I think that should be a separate patch if so.
I will have it removed. irrelevant to this change.


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:168
+                 (AArch64::AEK_CRC | AEK_CRYPTO | AEK_FP | AEK_SIMD | AEK_LSE | AEK_RAND))
+AARCH64_CPU_NAME("thunderx3t110", ARMV8_3A, FK_CRYPTO_NEON_FP_ARMV8, false,
+                 (AArch64::AEK_CRC | AEK_CRYPTO | AEK_FP | AEK_SIMD | AEK_LSE | AEK_RAND | AArch64::AEK_PROFILE | AArch64::AEK_RAS))
----------------
DavidSpickett wrote:
> Add a tests for this in llvm/unittests/Support/TargetParserTest.cpp (there are existing ones for "thunderx2t99").
> Might need to update "NumAArch64CPUArchs" too.
OK, we will have it fixed


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:849-857
+// These pointer authentication instructions require armv8.3a
+let Predicates = [HasV8_3a, HasPA] in {
 let Uses = [LR], Defs = [LR] in {
   def PACIAZ   : SystemNoOperands<0b000, "hint\t#24">;
   def PACIBZ   : SystemNoOperands<0b010, "hint\t#26">;
   let isAuthenticated = 1 in {
     def AUTIAZ   : SystemNoOperands<0b100, "hint\t#28">;
----------------
chill wrote:
> wxz2020 wrote:
> > wxz2020 wrote:
> > > ktkachov wrote:
> > > > IIRC these instructions are deliberately allowed in pre-armv8.3 targets because they are encoded in the NOP-space and can be deployed on pre-armv8.3 targets 
> > > I will do some research on this.
> > According to the documents, pointer authenticatoin got officially supporoted starting from armv8.3.  
> These instructions are executed as NOP on pre v8.3-A architectures. It allows you to have a single compatible binary that works correctly on pre v8.3-a (ofc, without pointer authentication), as well as on
> 8.3-a and later cores, with pointer authentication.
> 
> Please, remove the predicates.
Agree. Will remove it.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedThunderX3T110.td:13
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
----------------
SjoerdMeijer wrote:
> I don't intend to check the numbers here, but just curious if there's an optimisation guide if people are curious?
We will release a document about the details soon.


================
Comment at: llvm/test/CodeGen/AArch64/loop-micro-op-buffer-size-t110.ll:25
+; CHECK: %val5 = add nuw nsw i32 %counter, 10
+; CHECK-NOT: %val = add i32 %counter, 5
+; CHECK-NOT: %val = add i32 %counter, 6
----------------
SjoerdMeijer wrote:
> I guess there won't be another define %val, it will be %val6, so this CHECK-NOT will never match even if there's another add?
This test is for our internal use. Sorry, Will have it removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78129