add support for Cavium ThunderX ARM64 processors

[steleman]

Summary

This set of patches adds support for Cavium ThunderX ARM64 processors:

• ThunderX
• ThunderX T81
• ThunderX T83
• ThunderX T88

[steleman]

Support for Cavium ThunderX T8X ARM64 processors.

[rengolin]

Few inline comments. I'm adding a few people to review plus copying llvm-commits.

Next time, remember that if you don't do that, people don't see it. :)

cheers,
--renato

lib/Support/TargetParser.cpp

451 ↗

(On Diff #84937)

Please, add unit test for this and the new cores.

lib/Target/AArch64/AArch64.td

308 ↗

(On Diff #84937)

Isn't LSE 8.1? Do X and XT88 only implement 8.0 + LSE?

[javed.absar]

Hi:
Some comments specifically on the scheduler. Overall the sched-model looks good, although I don't have the detailed latency and micro-architecture information of ThunderX.

A few points you may want to consider :

1. You could remove the lines with 'let ResourceCycles = [1];' as that's by-default. That would help you reduce size of descriptions in a number of places.

2. Same for 'let Latency = 1;'

3. I don't know the micro-arch of ThunderX, but is there a typo for the latency of THXT8XWriteVST3 as it exceeds corresponding resource-cycle for the same sched-class. You may want to just double-check that.

4. ReadAdvance seems to exceed write latency in some cases, and so you may want to double check that - e.g. ReadI (2) verus WriteI (1).

Best Regards
Javed

[steleman]

In D28891#650425, @javed.absar wrote:

Hi:
Some comments specifically on the scheduler. Overall the sched-model looks good, although I don't have the detailed latency and micro-architecture information of ThunderX.

A few points you may want to consider :

1. You could remove the lines with 'let ResourceCycles = [1];' as that's by-default. That would help you reduce size of descriptions in a number of places.

2. Same for 'let Latency = 1;'

3. I don't know the micro-arch of ThunderX, but is there a typo for the latency of THXT8XWriteVST3 as it exceeds corresponding resource-cycle for the same sched-class. You may want to just double-check that.

4. ReadAdvance seems to exceed write latency in some cases, and so you may want to double check that - e.g. ReadI (2) verus WriteI (1).

Best Regards
Javed

Thank you very much for the comments - Renato and Javed.

I will add test cases, review the latencies and re-submit with corrections.

[MatzeB]

lib/Target/AArch64/AArch64Subtarget.cpp

90 ↗

(On Diff #84937)

HasLSE and HasV8_1aOps are already modeled as target features and should not set here. This switch exists because we cannot model integer (or any non-boolean) values with target features today. The rest should go to AArch64.td where you already added it anyway it seems.

[jgreenhalgh]

The feature bits set match my (limited) understanding of the various ThunderX variants, and their implementation in GCC. From that perspective this patch looks good to me.

I don't have any opinion on the pipeline model (I don't have access to a datasheet to check against), nor on how idiomatic this patch would be for LLVM (I'm not familiar with the code base).

[steleman]

Fixes and corrections as per review comments.
Added test case for ARMV8.1-A and LSE (test/MC/AArch64/armv8.1a-lse.s).

[steleman]

This latest changeset is based on ToT from 02/02/2017.

[rengolin]

Hi, can you also please add the relevant tests to unittests/Support/TargetParserTest.cpp? It should be pretty obvious from the context. Looks good otherwise.

[steleman]

In D28891#671863, @rengolin wrote:

Hi, can you also please add the relevant tests to unittests/Support/TargetParserTest.cpp? It should be pretty obvious from the context.

Yes I certainly can. Please stay tuned.

[steleman]

• Added tests for ARMV8.1-A LSE Extensions.
• Added tests in unittests/Support/TargetParserTest.cpp

[rengolin]

Thanks! LGTM.

[MatzeB]

lib/Target/AArch64/AArch64Subtarget.h

48–53

Please sort this alphabetically!

[steleman]

Sorted the ThunderX CPU names alphabetically in AArch64Subtarget.h.

[MatzeB]

In D28891#675163, @steleman wrote:

Sorted the ThunderX CPU names alphabetically in AArch64Subtarget.h.

Thanks, still good to go.

lib/Target/AArch64/AArch64Subtarget.h

     ExynosM1,
     Falkor,
     Kryo,
-    Vulcan
+    Vulcan,
+    ThunderX,
+    ThunderXT88,
+    ThunderXT81,
+    ThunderXT83
   };
 
 protected: