This is an archive of the discontinued LLVM Phabricator instance.

Add pipeline model for HiSilicon's TSV110
ClosedPublic

Authored by Elvina on Oct 22 2020, 10:09 AM.

Details

Summary

This patch adds the scheduling and cost model for TSV110.

Yours,
Elvina
Software Engineer
Advanced Software Technology Lab, Huawei

Diff Detail

Event Timeline

Elvina created this revision.Oct 22 2020, 10:09 AM
Elvina requested review of this revision.Oct 22 2020, 10:09 AM
Elvina edited the summary of this revision. (Show Details)Oct 23 2020, 12:11 AM

Failure on test "linux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp" looks bogus. I found the same cases https://reviews.llvm.org/D89895 and https://reviews.llvm.org/D89964.

Gentle ping :)

SjoerdMeijer added inline comments.Oct 29 2020, 5:07 AM
clang/test/Driver/aarch64-cpus.c
298 ↗(On Diff #300000)

This is unrelated to the scheduling model.
Looks like good tests to me, so please go ahead and commit this separately, no need for another review.

llvm/lib/Target/AArch64/AArch64SchedTSV110Details.td
1 ↗(On Diff #300000)

Just a nit/question on this new file: is there a benefit of having this separately? If there's none, just merge into AArch64SchedTSV110.td, as that would be more consistent with other schedmodels?

bryanpkc added inline comments.Oct 29 2020, 10:33 PM
llvm/lib/Target/AArch64/AArch64SchedTSV110.td
10

I suggest deleting the word ARM here to avoid confusion.

29

Indent to align with the previous argument?

54

Nit: Leave a space after the // delimiter.

129

Type: Refinements

Elvina marked 6 inline comments as done.Oct 29 2020, 11:58 PM
Elvina added inline comments.
clang/test/Driver/aarch64-cpus.c
298 ↗(On Diff #300000)

Will do, thanks

llvm/lib/Target/AArch64/AArch64SchedTSV110Details.td
1 ↗(On Diff #300000)

No specific needs to do this, so I'll merge all into one file.

Elvina updated this revision to Diff 301821.Oct 29 2020, 11:58 PM
Elvina marked 2 inline comments as done.

Merged all into one AArch64SchedTSV110.td, removed aarch64-cpus.c test from this patch

SjoerdMeijer accepted this revision.Oct 30 2020, 1:09 AM

I haven't checked the instruction descriptions in detail, but the overall structure looks good to me. Perhaps wait a day with committing in case @bryanpkc has more comments.

This revision is now accepted and ready to land.Oct 30 2020, 1:09 AM
Elvina added a comment.Nov 5 2020, 1:28 AM

@SjoerdMeijer thanks for the review!
@bryanpkc does everything look fine? Can I commit it?

This revision was automatically updated to reflect the committed changes.