[PATCH] D24540: [AArch64] Add Cavium ThunderX subtarget support.

[Joel Jones via llvm-commits]

joelkevinjones added inline comments.


================
Comment at: lib/Target/AArch64/AArch64.td:267
+                                   FeaturePostRAScheduler,
+				   HasV8_1aOps]>;
+
----------------
jgreenhalgh wrote:
> rengolin wrote:
> > davidlt wrote:
> > > rengolin wrote:
> > > > joelkevinjones wrote:
> > > > > jgreenhalgh wrote:
> > > > > > davidlt wrote:
> > > > > > > rengolin wrote:
> > > > > > > > jgreenhalgh wrote:
> > > > > > > > > jmolloy wrote:
> > > > > > > > > > Indentation is off here.
> > > > > > > > > I'm confused by this feature bit, and it doesn't line up with the -mcpu=thunderx support in GCC.
> > > > > > > > > 
> > > > > > > > > As far as I am aware, HasV8_1aOps enables support for all of the ARMv8.1-A extensions, i.e. the Large System Extensions, PAN, and the RDMA instructions. But, there exist ThunderX implementations which don't provide support for all of these extensions. For these ThunderX targets, the set of extensions enabled here would be too large.
> > > > > > > > > 
> > > > > > > > > The GCC implementation only enables ARMv8-A with the CRC, Advanced SIMD, Floating-Point and Cryptography extensions.
> > > > > > > > > 
> > > > > > > > > Would that be more correct here?
> > > > > > > > If that's the case, then yes, ARMv8A would be a better match. I don't know enough about ThunderX support to have an opinion, though.
> > > > > > > IIRC, ThunderX Pass 1 was ARMv8.0 silicon (before landed in general availability products, I guess), but what's shipping now is ThunderX Pass 2 which is ARMv8.1.
> > > > > > > 
> > > > > > > From Andrew Pinski patch (Nov 2015) which probably never landed in GCC:
> > > > > > > 
> > > > > > > 
> > > > > > > > The reason why thunderxt88pass1 is seperate from thunderx is because
> > > > > > > > thunderx is changed to be an ARMv8.1 arch core while thunderxt88pass1
> > > > > > > > is still an ARMv8 arch core.
> > > > > > > 
> > > > > > > See, e.g. https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02148.html
> > > > > > OK. My memory was that there was no ThunderX part which supported the RDMA AdvancedSIMD extensions, only the Large System Extensions. But that GCC patch would have enabled the same set of extensions and would bring LLVM and GCC back in line. I'm happy to withdraw my objection.
> > > > > That is correct. Although there were very few pass 1 machines that "escaped", I will redo the patch to have a means of specifying explicitly that ones wants to generate code for a pass 1 thunderx. Would it be best to (is it even possible) in AArch64.td to use a SubtargetFeature<"pass1", ... to disable features from a base of "thunderx"?
> > > > No, they should be two different cpus: "thunderx1" and thunderx2".
> > > > 
> > > > Or we only implement "thunderx" as "thunderx2" and tell those few people that have pass1 to use:
> > > > 
> > > >     -march=armv8-a+crc+crypto+nolse+nordma+nopan
> > > You don't want to use "thunderx2" for "pass 2" silicons. Cavium already announced 2nd gen silicons aka "thunderx2", those are ARMv8.2. You can read more here: http://cavium.com/newsevents-Cavium-Announces-ThunderX2.html
> > Hum... And I thought X2 was the same as pass 2... :/
> > 
> > Right, than I suggest you just leave the pass 1 people in the dark and make thunderx 8.1, requiring the convoluted march command line to the few damned souls.
> > 
> > Isn't that what happens in GCC, too?
> Not until someone updates the support for -mcpu=thunderx to be ThunderX Pass 2. At the moment GCC can only target ThunderX Pass 1, and you need the command line dance to get LSE extensions (and others) enabled for Pass 2.
Not having to do a separate sub-target works for me. :-)

Is there a good place to place user documentation for the pass 1 dance?


Repository:
  rL LLVM

https://reviews.llvm.org/D24540