Add the scheduling and cost model for Exynos M3.
Details
- Reviewers
jmolloy kristof.beyls MatzeB rengolin - Commits
- rG9f9daa1f14f4: [AArch64] Add pipeline model for Exynos M3
rG07c78eeeef61: [AArch64] Add new target feature to handle cheap as move for Exynos
rL323774: [AArch64] Add new target feature to handle cheap as move for Exynos
rL323773: [AArch64] Add pipeline model for Exynos M3
Diff Detail
- Repository
- rL LLVM
Event Timeline
I can't say much about the scheduling model itself as I'm not familiar with Exynos.
LGTM if you introduce a subtarget feature for the isCheapAsAMove behavior (see below).
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | ||
---|---|---|
678โ679 โ | (On Diff #130931) | Looks like it is time to make a subtarget feature out of this. |
llvm/test/CodeGen/AArch64/misched-fusion-aes.ll | ||
1โ9 โ | (On Diff #130931) | More candidates here where at some point we better start testing -mattr=+fuse-aes / -mattr=-fuse-aes and have a seperate test to check target feature lists of CPUs... (but as I noted in my other review that is work for another time/commit). |
Actually can you rename the scheduling model to AArch64SchedExynosM3.td? (Feel free to do the same with the M1 file in a separate commit without review).
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | ||
---|---|---|
678โ679 โ | (On Diff #130931) | I agree in principle, but there already is FeatureCustomCheapAsMoveHandling, so I'm afraid that it might be a bit more involved and that it perhaps deserves a separate discussion. Basically, these checks customize the custom handling of isAsCheapAsAMove because Exynos is different from the other AArch64 targets. |
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | ||
---|---|---|
678โ679 โ | (On Diff #130931) | On a general level: I really think we should push towards not using getProcFamily() outside AArch64Subtarget.cpp; It's a sign that things aren't modeled cleanly and it'll only get worse with new exception added. And while I wouldn't mind a case like this in isolation, in practice people copy the patterns they see elsewhere and I don't want this to be the copy&paste source for more getProcFamily() calls in the target. Adding FeatureExynosCheapAsMoveHandling (or a better name if you have one) and using that here shouldn't be that hard. It's unfortunate that it would only kick in when CustomCheapAsMoveHandling is enabled too, but not a dealbreaker or reason not introduce the subtarget feature here. |
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | ||
---|---|---|
678โ679 โ | (On Diff #130931) | I had to point it out, but, if it sounds sensible to you, I do not object to it. |