This is an archive of the discontinued LLVM Phabricator instance.

[ARM][TEST] Add a new test case of add-imm & sub-imm
ClosedPublic

Authored by benshi001 on Jul 15 2020, 9:12 PM.

Details

Summary

This case will show furture optimization of add-imm & sub-imm.

Diff Detail

Event Timeline

benshi001 created this revision.Jul 15 2020, 9:12 PM
MaskRay added inline comments.Jul 15 2020, 10:12 PM
llvm/test/CodeGen/ARM/armv6-add-sub-imm.ll
1

Have you considered removing armv6- from the filename?

If people want to test armv7-, they can simply add another RUN line.

BTW, you may consider llvm/utils/update_llc_test_checks.py, which can make test updating easy.

6

; CHECK-LABEL: sub0:

17

Add -NEXT: whenever appropriate

benshi001 retitled this revision from [ARM][TEST] Add a new test case of add-imm & sub-imm on armv6 to [ARM][TEST] Add a new test case of add-imm & sub-imm.
benshi001 edited the summary of this revision. (Show Details)
benshi001 marked 3 inline comments as done.
MaskRay added inline comments.Jul 16 2020, 8:52 AM
llvm/test/CodeGen/ARM/add-sub-imm.ll
4 ↗(On Diff #278379)

;; probably works better. In the future, we can ask update_llc_test_checks.py to always retain ;; comments. (There is some intricate logic retaining ; comments)

benshi001 updated this revision to Diff 278626.Jul 16 2020, 4:22 PM
benshi001 marked an inline comment as done.
benshi001 updated this revision to Diff 279072.Jul 19 2020, 5:07 AM

Although I don't mind at all new tests being added, in this case I am really wondering if the code paths are not exercised by existing tests. And also reading the description:

This case will show furture optimization of add-imm & sub-imm.

I am wondering what these future optimizations are and if these tests should simply go in with that then?

Although I don't mind at all new tests being added, in this case I am really wondering if the code paths are not exercised by existing tests. And also reading the description:

This case will show furture optimization of add-imm & sub-imm.

I am wondering what these future optimizations are and if these tests should simply go in with that then?

I expect this test will show improvements in
https://reviews.llvm.org/D83745
https://reviews.llvm.org/D84100

I hope this test case first be merged, then the above two patches will affect this test.

SjoerdMeijer accepted this revision.Jul 28 2020, 11:18 AM
This revision is now accepted and ready to land.Jul 28 2020, 11:18 AM
SjoerdMeijer closed this revision.Jul 29 2020, 5:32 AM

Committed in rG85342c27a303 as part of D83745.