[PATCH] D78129: Add Marvell ThunderX3T110 support

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:849
+                                          HasV8_1aOps,
+                                          HasV8_3aOps]>;
+
----------------
`HasV8_3aOps` implies `HasV8_2aOps`, which implies `HasV8_1aOps`.
So you can just remove `HasV8_1aOps`? 


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedThunderX3T110.td:13
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
----------------
I don't intend to check the numbers here, but just curious if there's an optimisation guide if people are curious?


================
Comment at: llvm/test/CodeGen/AArch64/loop-micro-op-buffer-size-t110.ll:1
+; REQUIRES: asserts
+; RUN: opt -mcpu=thunderx3t110 -loop-unroll --debug-only=loop-unroll --debug-only=basicblock-utils -S -unroll-allow-partial < %s 2>&1 | FileCheck %s
----------------
Couple of question about this test.
Looks like you're both testing output on stdout and stderr, and your checks for this are interleaved. If I am not mistaken, the behaviour of this can vary on different platforms, and thus may fail on different platforms.

But more importantly, what is this test exactly testing? The file name gives away a hint of course, but I don't see yet how this interact with loop unrolling. Is the loop unroller looking at MaxInterleaveFactor that is set here, but is that what we are testing here?


================
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
----------------
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?


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

https://reviews.llvm.org/D78129