This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Use Neoverse N2 sched model as default for:
ClosedPublic

Authored by c-rhodes on Jul 6 2022, 7:38 AM.

Download Raw Diff

Details

Reviewers

paulwalker-arm
dmgreen
peterwaller-arm
bsmith
andreadb

Commits

rGd1c51d45f080: [AArch64] Use Neoverse N2 sched model as default for:

Summary

Cortex-A710
Cortex-X2
Neoverse-V1
Neoverse-512tvb

Diff Detail

Unit TestsFailed

	Time	Test
	60,090 ms	x64 debian > AddressSanitizer-x86_64-linux-dynamic.TestCases::scariness_score_test.cpp
	60,100 ms	x64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
	330 ms	x64 debian > BOLT.runtime/X86::user-func-reorder.c
	60,030 ms	x64 debian > MLIR.Examples/standalone::test.toy

Event Timeline

c-rhodes created this revision.Jul 6 2022, 7:38 AM

Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2022, 7:38 AM

Herald added subscribers: hiraditya, kristof.beyls. · View Herald Transcript

c-rhodes requested review of this revision.Jul 6 2022, 7:38 AM

Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2022, 7:38 AM

c-rhodes added a parent revision: D128631: [AArch64] Initial sched model for Neoverse N2.Jul 6 2022, 7:38 AM

c-rhodes mentioned this in D128631: [AArch64] Initial sched model for Neoverse N2.

c-rhodes added inline comments.Jul 6 2022, 7:42 AM

llvm/lib/Target/AArch64/AArch64.td
1157–1160	@dmgreen Is it ok to use the N2 as the default model for these cores given they're not Armv9 and only support SVEv1?

Matt added a subscriber: Matt.Jul 6 2022, 8:27 AM

Harbormaster completed remote builds in B173895: Diff 442566.Jul 6 2022, 8:30 AM

Thanks for the patch. It may be possible to write a quick test that the scheduling info for one of the new instructions.

Otherwise LGTM.

llvm/lib/Target/AArch64/AArch64.td
1157–1160	Yeah - I think that's OK. The model will just have more coverage than the core. It should always be a better fit than the A57 model. The extra info in the schedules will just be unused.

This revision is now accepted and ready to land.Jul 7 2022, 12:43 PM

Add simple tests to check N2 model is being used.

Herald added a reviewer: andreadb. · View Herald TranscriptJul 8 2022, 3:46 AM

Herald added a subscriber: gbedwell. · View Herald Transcript

In D129203#3636861, @dmgreen wrote:

Thanks for the patch. It may be possible to write a quick test that the scheduling info for one of the new instructions.

Otherwise LGTM.

Cheers Dave, I've added some tests.

Harbormaster completed remote builds in B174349: Diff 443195.Jul 8 2022, 4:36 AM

This revision was landed with ongoing or failed builds.Jul 8 2022, 6:34 AM

Closed by commit rGd1c51d45f080: [AArch64] Use Neoverse N2 sched model as default for: (authored by c-rhodes). · Explain Why

This revision was automatically updated to reflect the committed changes.

c-rhodes added a commit: rGd1c51d45f080: [AArch64] Use Neoverse N2 sched model as default for:.

Revision Contents

Path

Size

llvm/

lib/

Target/

AArch64/

AArch64.td

8 lines

Diff 442566

llvm/lib/Target/AArch64/AArch64.td

Context not available.
	[TuneA78]>;	[TuneA78]>;
	def : ProcessorModel<"cortex-a78c", CortexA57Model, ProcessorFeatures.A78C,	def : ProcessorModel<"cortex-a78c", CortexA57Model, ProcessorFeatures.A78C,
	[TuneA78C]>;	[TuneA78C]>;
	def : ProcessorModel<"cortex-a710", CortexA57Model, ProcessorFeatures.A710,	def : ProcessorModel<"cortex-a710", NeoverseN2Model, ProcessorFeatures.A710,
	[TuneA710]>;	[TuneA710]>;
	def : ProcessorModel<"cortex-r82", CortexA55Model, ProcessorFeatures.R82,	def : ProcessorModel<"cortex-r82", CortexA55Model, ProcessorFeatures.R82,
	[TuneR82]>;	[TuneR82]>;
Context not available.
	[TuneX1]>;	[TuneX1]>;
	def : ProcessorModel<"cortex-x1c", CortexA57Model, ProcessorFeatures.X1C,	def : ProcessorModel<"cortex-x1c", CortexA57Model, ProcessorFeatures.X1C,
	[TuneX1]>;	[TuneX1]>;
	def : ProcessorModel<"cortex-x2", CortexA57Model, ProcessorFeatures.X2,	def : ProcessorModel<"cortex-x2", NeoverseN2Model, ProcessorFeatures.X2,
	[TuneX2]>;	[TuneX2]>;
	def : ProcessorModel<"neoverse-e1", CortexA53Model,	def : ProcessorModel<"neoverse-e1", CortexA53Model,
	ProcessorFeatures.NeoverseE1, [TuneNeoverseE1]>;	ProcessorFeatures.NeoverseE1, [TuneNeoverseE1]>;
Context not available.
	ProcessorFeatures.NeoverseN1, [TuneNeoverseN1]>;	ProcessorFeatures.NeoverseN1, [TuneNeoverseN1]>;
	def : ProcessorModel<"neoverse-n2", NeoverseN2Model,	def : ProcessorModel<"neoverse-n2", NeoverseN2Model,
	ProcessorFeatures.NeoverseN2, [TuneNeoverseN2]>;	ProcessorFeatures.NeoverseN2, [TuneNeoverseN2]>;
	def : ProcessorModel<"neoverse-512tvb", CortexA57Model,	def : ProcessorModel<"neoverse-512tvb", NeoverseN2Model,
	ProcessorFeatures.Neoverse512TVB, [TuneNeoverse512TVB]>;	ProcessorFeatures.Neoverse512TVB, [TuneNeoverse512TVB]>;
	def : ProcessorModel<"neoverse-v1", CortexA57Model,	def : ProcessorModel<"neoverse-v1", NeoverseN2Model,
	ProcessorFeatures.NeoverseV1, [TuneNeoverseV1]>;	ProcessorFeatures.NeoverseV1, [TuneNeoverseV1]>;
		c-rhodesAuthorUnsubmitted Not Done Reply Inline Actions @dmgreen Is it ok to use the N2 as the default model for these cores given they're not Armv9 and only support SVEv1? c-rhodes: @dmgreen Is it ok to use the N2 as the default model for these cores given they're not Armv9…
		dmgreenUnsubmitted Not Done Reply Inline Actions Yeah - I think that's OK. The model will just have more coverage than the core. It should always be a better fit than the A57 model. The extra info in the schedules will just be unused. dmgreen: Yeah - I think that's OK. The model will just have more coverage than the core. It should…
	def : ProcessorModel<"exynos-m3", ExynosM3Model, ProcessorFeatures.ExynosM3,	def : ProcessorModel<"exynos-m3", ExynosM3Model, ProcessorFeatures.ExynosM3,
	[TuneExynosM3]>;	[TuneExynosM3]>;
Context not available.