This is an archive of the discontinued LLVM Phabricator instance.

[Sanitizer][Apple] Enable sanitizer common unittests for arm64 archs on Apple
ClosedPublic

Authored by thetruestblue on Jan 6 2023, 4:31 PM.

Details

Summary

This patch enables sanitizer common unit tests for arm64 architecture only on apple devices.

Also lowers the expected compression ratio for 64 bit machines. Unsure of justification of 130. On apple arm64 we're seeing this number comeout to 128.6

rdar://101436019

Diff Detail

Event Timeline

thetruestblue created this revision.Jan 6 2023, 4:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2023, 4:31 PM
thetruestblue requested review of this revision.Jan 6 2023, 4:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2023, 4:31 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
rsundahl requested changes to this revision.Jan 9 2023, 3:15 PM
rsundahl added inline comments.
compiler-rt/lib/sanitizer_common/tests/sanitizer_stack_store_test.cpp
148

I think it'd be better to keep the test but reduce the 30% either for all targets or just for Apple arm64. I say this because it doesn't appear to be a test for the efficiency of the compression but rather that the compression is happening at all. (Nit: The comment is: "slightly than expected" but should be "slightly less than expected")

This revision now requires changes to proceed.Jan 9 2023, 3:15 PM

Rather than skip the test, lower expected ratio. Per @rsundahl 's suggestion.

thetruestblue edited the summary of this revision. (Show Details)Jan 10 2023, 11:36 AM
thetruestblue added reviewers: MaskRay, kstoimenov.
thetruestblue marked an inline comment as done.
MaskRay accepted this revision.Jan 10 2023, 11:56 AM

The use site is EXPECT_GE(kBlockSizeBytes / (kBlockSizeBytes - (before - after)), expected_ratio);

Lowering the number loosens the test but it should be fine.

rsundahl accepted this revision.Jan 10 2023, 12:35 PM

LGTM. We can probably improve that ratio with some dictionary customization at some point. Thanks for the look @MaskRay.

This revision is now accepted and ready to land.Jan 10 2023, 12:35 PM
vitalybuka accepted this revision.Jan 10 2023, 12:48 PM
kstoimenov accepted this revision.Jan 10 2023, 12:52 PM