This is an archive of the discontinued LLVM Phabricator instance.

[NFC][AArch64] Specify aarch64 little-endian linux datalayout explicitly for cast.ll and arith-overflow.ll
ClosedPublic

Authored by mingmingl on Aug 29 2022, 8:19 AM.

Details

Summary

These two tests stand out from a sweep study (D132889) to see how aarch64-little-endian linux data layout affects test results.

This is the pre-commit test for D132784

Diff Detail

Event Timeline

mingmingl created this revision.Aug 29 2022, 8:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 8:19 AM
mingmingl requested review of this revision.Aug 29 2022, 8:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 8:19 AM

move target layout from opt option to IR, so that only one DL is needed per file (not one DL per RUN)

mingmingl retitled this revision from [NFC][AArch64] Specify datalayout explicitly for llvm/test/Analysis/CostModel/AArch64/cast.ll to [NFC][AArch64] Specify datalayout explicitly for cast.ll and arith-overflow.ll .
mingmingl edited the summary of this revision. (Show Details)

Add arith-overflow.ll into diffbase.

mingmingl retitled this revision from [NFC][AArch64] Specify datalayout explicitly for cast.ll and arith-overflow.ll to [NFC][AArch64] Specify aarch64 little-endian linux datalayout explicitly for cast.ll and arith-overflow.ll .Aug 30 2022, 12:49 AM
mingmingl edited the summary of this revision. (Show Details)
fhahn accepted this revision.Aug 30 2022, 1:23 AM

LGTM, thanks!

This revision is now accepted and ready to land.Aug 30 2022, 1:23 AM
mingmingl closed this revision.Aug 30 2022, 9:47 AM

thanks! Pushed https://github.com/llvm/llvm-project/commit/3785234b03c568cd135b18c79cd7606108b9d388.

Need to manually close this (uploaded commit message doesn't have differential revision link)

Thanks. This does sound good. It is probably worth doing it for all the other tests, as in D132889 even if they don't alter the costs.

Thanks. This does sound good. It is probably worth doing it for all the other tests, as in D132889 even if they don't alter the costs.

Adding them makes sense! Sent out D132889.