[AArch64] Add Cavium ThunderX subtarget support.
Add subtarget detail and scheduling model for Cavium ThunderX core (ARMv8.1A).
Differential D24540
[AArch64] Add Cavium ThunderX subtarget support. joelkevinjones on Sep 13 2016, 6:30 PM. Authored by
Details [AArch64] Add Cavium ThunderX subtarget support. Add subtarget detail and scheduling model for Cavium ThunderX core (ARMv8.1A).
Diff Detail
Event TimelineComment Actions There should probably be MC tests for the v8.1a features, but other than that the non-scheduler bits look fine (we'll have to take your word on the scheduler). Comment Actions Please, add TargetParser unit tests, to make sure the correct options are selected, but otherwise, also looks good to me. Comment Actions Seems a little light on testing, but then again there's not a lot of reason to add tons of identical testing (though we have it right now). TargetParser does need tests. Probably wants a clang patch as well? -eric
Comment Actions I'm going to be working on this patch from Ananth. For the AsmParser test, I'm assuming that the file being referred to is: unittests/Support/TargetParserTest.cpp? If so, I have several questions:
Comment Actions Hi, This generally looks good to me - I agree that the targetparser testing is pretty tautological. Eric, do you have input on whether that's needed? Cheers, James
Comment Actions We also have some parsing tests to make sure we're not loosing anything, but I agree, the number of relevant tests there is not great. I think this can be a task for a separate patch. cheers,
Comment Actions
I've done a search in both test and unittests for some kind of parsing test for features present vs. not present, but I could find what you were referring to.
Comment Actions I'm splitting this revision into two parts, One will be to have the target names match those in the GCC proposal (see the gcc-patches mailing list, 1 November 2016, https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00155.html). The other will be a fix to the instruction mapping to scheduling information. |
Indentation is off here.