This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add Fujitsu A64FX scheduling model
ClosedPublic

Authored by kawashima-fj on Dec 23 2020, 6:25 PM.

Details

Summary

Basic support of A64FX was added in D75594 but its scheduling model
was missing. This commit adds the scheduling model. Also, this commit
amends/adds some subtarget parameters of A64FX.

The A64FX Microarchitecture Manual, which is source information of
this commit, is on GitHub.

https://github.com/fujitsu/A64FX/

I'm on holiday till January 4. I'll address comments and commit
to GitHub after that.

Diff Detail

Event Timeline

kawashima-fj created this revision.Dec 23 2020, 6:25 PM
kawashima-fj requested review of this revision.Dec 23 2020, 6:25 PM
yutsumi added inline comments.Dec 23 2020, 7:01 PM
llvm/lib/Target/AArch64/AArch64Subtarget.cpp
116–117

Please remove this comment because LoopDataPrefetch is enabled by default in llvm/lib/Target/AArch64/AArch64TargetMachine.cpp.

@yutsumi Thanks. Good catch. I've addressed your comment.

yutsumi accepted this revision.Dec 23 2020, 8:53 PM

@kawashima-fj Thanks. LGTM.

This revision is now accepted and ready to land.Dec 23 2020, 8:53 PM
dmgreen added inline comments.
llvm/lib/Target/AArch64/AArch64SchedA64FX.td
103

Is this used anywhere? If so, for what?

llvm/lib/Target/AArch64/AArch64Schedule.td
35 ↗(On Diff #313648)

What is this for? It doesn't appear to be used in any instructions.

Is there a way to specify the maximum or even the set of supported SIMD-widths for SVE cores?

kawashima-fj marked an inline comment as done.
kawashima-fj set the repository for this revision to rG LLVM Github Monorepo.

Remove WriteID512 and update comment.

kawashima-fj marked an inline comment as done.Jan 4 2021, 7:57 PM
kawashima-fj added inline comments.
llvm/lib/Target/AArch64/AArch64SchedA64FX.td
103

A64FXAny is not used elsewhere explicitly but removing it causes the following TableGen error. CodeGenSchedModels::verifyProcResourceGroups function in llvm/utils/TableGen/CodeGenSchedule.cpp emits this error.

$ $build/NATIVE/bin/llvm-tblgen -gen-subtarget
    -I $src/llvm/lib/Target/AArch64 -I$build/include -I$src/llvm/include
    -I $src/llvm/lib/Target $src/llvm/lib/Target/AArch64/AArch64.td
    --write-if-changed -o lib/Target/AArch64/AArch64GenSubtargetInfo.inc
    -d lib/Target/AArch64/AArch64GenSubtargetInfo.inc.d
Included from $src/llvm/lib/Target/AArch64/AArch64.td:556:
$src/llvm/lib/Target/AArch64/AArch64SchedA64FX.td:80:1:
  error: proc resource group overlaps with A64FXGI02 but no supergroup contains both.
def A64FXGI01 : ProcResGroup<[A64FXIPFLA, A64FXIPPR]>;
^

Does anyone know why this overlaping check exists?

llvm/lib/Target/AArch64/AArch64SchedThunderX2T99.td also has THX2T99Any and llvm/lib/Target/X86/X86Sched*.td also have *PortAny. I copied it.

llvm/lib/Target/AArch64/AArch64Schedule.td
35 ↗(On Diff #313648)

It was my mistake. WriteID512 was not needed. I removed it in the latest patch. Thanks.

kawashima-fj marked an inline comment as done.Jan 4 2021, 8:01 PM

Is there a way to specify the maximum or even the set of supported SIMD-widths for SVE cores?

@tschuett
I think there is no field to specify the maximum SIMD-width in the machine model.

llvm/lib/Target/AArch64/AArch64Subtarget.cpp has cl::opt<unsigned> SVEVectorBitsMax variable. Specifying -aarch64-sve-vector-bits-max=... option can set the value at the application compile time. However, the SVEVectorBitsMax is not used anywhere in the LLVM code currently.

What is the intent of the question? Suggestion to specify it in the machine model? If so, we can add the field when a usage comes up.

Yes, if it could be part of the machine model. It is specific to your core.

dmgreen accepted this revision.Jan 6 2021, 12:59 AM

Thanks. I don't know any of the details of this specific core, but this LGTM in terms of a schedule.

If so, we can add the field when a usage comes up.

Sounds good to me. It might well be the kind of thing we specify with a subtarget feature or the same as how Loop/Function Alignments are specified.

Thanks for reviews! I'll commit this next week unless other new comment is posted.

This revision was automatically updated to reflect the committed changes.