This is an archive of the discontinued LLVM Phabricator instance.

[LoopDataPrefetch] Fix crash when TTI doesn't set CacheLineSize
ClosedPublic

Authored by Allen on Jul 23 2022, 4:12 AM.

Diff Detail

Event Timeline

Allen created this revision.Jul 23 2022, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2022, 4:12 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
Allen requested review of this revision.Jul 23 2022, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2022, 4:12 AM

please can you add the test case ?

Allen updated this revision to Diff 447172.Jul 24 2022, 7:00 PM

Add test case

please can you add the test case ?

Done, thanks @RKSimon

RKSimon added inline comments.
llvm/test/Transforms/LoopDataPrefetch/AArch64/pr56681.ll
6

If you're going to check a debug message, I think you need "; REQUIRES: asserts" ?

Do you force all targets to provide PrefetchDistance and PrefetchDistance or could it be an llvm::optional ?

Allen added a comment.Jul 25 2022, 4:22 AM

Do you force all targets to provide PrefetchDistance and PrefetchDistance or could it be an llvm::optional ?

Thanks, I think it is unfriendly to crash at the moment. Use a default value when they are not specified can be independent next step, if you feel the need.

RKSimon added inline comments.Jul 25 2022, 4:23 AM
llvm/test/Transforms/LoopDataPrefetch/AArch64/pr56681.ll
2

Is the update script necessary? Did it manage to generate up the CHECK ?

Allen updated this revision to Diff 447274.Jul 25 2022, 4:31 AM
Allen updated this revision to Diff 447275.Jul 25 2022, 4:33 AM
RKSimon accepted this revision.Jul 25 2022, 8:40 AM

LGTM - ideally we'd still check some IR (even if its just a ret void)

This revision is now accepted and ready to land.Jul 25 2022, 8:40 AM
Allen updated this revision to Diff 447563.Jul 25 2022, 10:03 PM
Allen marked 2 inline comments as done.

delete option -disable-output

This revision was landed with ongoing or failed builds.Jul 25 2022, 10:09 PM
This revision was automatically updated to reflect the committed changes.