This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Remove obsoleted feature
AcceptedPublic

Authored by evandro on Nov 15 2017, 4:04 PM.

Details

Reviewers
az
mcrosier
fhahn
Summary

By instead relying on the cost model on whether loads and stores should be folded in AArch64LoadStoreOpt by D39976, FeatureSlowPaired128 became unnecessary.

Diff Detail

Repository
rL LLVM

Event Timeline

evandro created this revision.Nov 15 2017, 4:04 PM
fhahn added a subscriber: fhahn.Nov 16 2017, 2:36 AM
fhahn added inline comments.
llvm/test/CodeGen/AArch64/arm64-ldp-cluster.ll
3

Would it make sense to keep this check? It looks like it tests a code path that relied on Paired128IsSlow (@ldq_cluster) and it should behave the same now on exynos-m1/m2, without the subtarget feature.

mcrosier added inline comments.Nov 16 2017, 5:53 AM
llvm/test/CodeGen/AArch64/arm64-ldp-cluster.ll
3

+1

This should be a NFC after D39976, right?

llvm/test/CodeGen/AArch64/no-quad-ldp-stp.ll
1

I'd suggest we keep this file, but drop this RUN command since the slow-paired-128 feature will no longer exist.

evandro marked 3 inline comments as done.Nov 16 2017, 9:15 AM
evandro added inline comments.
llvm/test/CodeGen/AArch64/arm64-ldp-cluster.ll
3

Correct.

3

You have a point.

evandro marked an inline comment as done.Nov 17 2017, 9:26 AM
evandro added inline comments.
llvm/test/CodeGen/AArch64/no-quad-ldp-stp.ll
1

The motivation to remove this file is that slow-paired-128 is too blunt and, with the more accurate evaluation in D39976, this test is superseded by the more complete test/CodeGen/AArch64/ldst-opt.ll.

evandro updated this revision to Diff 123362.Nov 17 2017, 9:28 AM

Restore the test case.

evandro marked 3 inline comments as done.Nov 17 2017, 9:29 AM
evandro updated this revision to Diff 124429.Nov 27 2017, 11:40 AM
evandro set the repository for this revision to rL LLVM.
fhahn accepted this revision.Nov 27 2017, 3:37 PM

LGTM, given that happy with the slight change in behavior in llvm/test/CodeGen/AArch64/arm64-ldp-cluster.ll for exynos.

llvm/test/CodeGen/AArch64/arm64-ldp-cluster.ll
2

Are there any remaining CORTEX or EXYNOS check lines in this file?

This revision is now accepted and ready to land.Nov 27 2017, 3:37 PM
evandro marked an inline comment as done.Nov 28 2017, 8:25 AM
evandro added inline comments.
llvm/test/CodeGen/AArch64/arm64-ldp-cluster.ll
2

Nope. Will update it.

Thank you.

evandro marked 2 inline comments as done.Dec 11 2017, 12:38 PM