Page MenuHomePhabricator

[test] Skip ThinLTO cache tests requiring atime setting on NetBSD
ClosedPublic

Authored by mgorny on Dec 4 2018, 8:07 AM.

Details

Summary

Skip the ThinLTO cache tests on NetBSD. They require 'touch' being
able to alter atime of files, while NetBSD inhibits atime updates
when filesystem is mounted noatime.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Dec 4 2018, 8:07 AM
krytarowski added inline comments.Dec 4 2018, 8:08 AM
test/ThinLTO/X86/cache.ll
2 ↗(On Diff #176641)

Please include an inline comment why unsupported.

mgorny updated this revision to Diff 176645.Dec 4 2018, 8:25 AM
mgorny marked an inline comment as done.

Updated wrt comment.

What would happen if the thin LTO cache is used on such a system? Do we need to put something in the thin LTO code to emit some sort of error, if this is attempted?

mgorny added a comment.Dec 4 2018, 9:03 AM

I don't really know; I suppose the same thing as if it were used on any regular system with noatime. However, I believe NetBSD is only special here as we can't set up the test environment with past dates.

krytarowski accepted this revision.Dec 4 2018, 9:59 AM
This revision is now accepted and ready to land.Dec 4 2018, 9:59 AM

What would happen if the thin LTO cache is used on such a system? Do we need to put something in the thin LTO code to emit some sort of error, if this is attempted?

Is this only optimization (i.e. not about generating correct result)? If so we will ignore it and align the NetBSD kernel behavior to Linux/FreeBSD.

jhenderson accepted this revision.Dec 5 2018, 2:24 AM

LGTM. Thinking about it a bit more, and I don't think using the cache will cause any actual problems if everything has the same access time, beyond potentially doing unnecessary work.

test/ThinLTO/X86/cache.ll
1 ↗(On Diff #176645)

Please move this comment down to the first set of RUN commands, since that's what it's describing. The UNSUPPORTED directive and associated comment are best kept at the top of the file before anything else.

mgorny updated this revision to Diff 176784.Dec 5 2018, 3:17 AM
mgorny marked an inline comment as done.
This revision was automatically updated to reflect the committed changes.