This is an archive of the discontinued LLVM Phabricator instance.

[libc] Disable 'DecodeInOtherBases` test on GPU targets
ClosedPublic

Authored by jhuber6 on Jul 21 2023, 9:20 AM.

Details

Summary

This test is excessively slow on GPU targets, taking anywhere beween 5
and 60 seconds to complete each time it's run. See
https://lab.llvm.org/buildbot/#/builders/55/builds/52203/steps/12/logs/stdio
for an example on the NVPTX buildbot. Simply disable testing this on the
GPU for now.

Diff Detail

Event Timeline

jhuber6 created this revision.Jul 21 2023, 9:20 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 21 2023, 9:20 AM
jhuber6 requested review of this revision.Jul 21 2023, 9:20 AM

Try disabling just the third set of loops, since the size of the checks grows exponentially it's likely that they're taking up most of the time.

jhuber6 updated this revision to Diff 542981.Jul 21 2023, 10:19 AM

Changing to limit the nested loop depth

I'd rather not limit the digits to just 0 and 1. If you want to shorten the tests maybe try 16? Otherwise probably just disable the whole test. Having the test enabled and running but not actually checking anything seems like a recipe for confusion.

I'd rather not limit the digits to just 0 and 1. If you want to shorten the tests maybe try 16? Otherwise probably just disable the whole test. Having the test enabled and running but not actually checking anything seems like a recipe for confusion.

Well we at least get the first loop's checks. This version takes 5ms on my machine, the original takes 6000ms, changing it to 16 takes 500ms.

How long does it take with a limit of 36 but disabling the loop that goes to the third digit?

How long does it take with a limit of 36 but disabling the loop that goes to the third digit?

100ms

I think that's the option I would prefer then. That way we get to test all of the 1 and 2 digit cases

jhuber6 updated this revision to Diff 542994.Jul 21 2023, 10:59 AM

Disable only tertiary loop

michaelrj accepted this revision.Jul 21 2023, 11:13 AM
michaelrj added inline comments.
libc/test/src/stdlib/StrtolTest.h
202

nit: Clarify this comment with the new design

This revision is now accepted and ready to land.Jul 21 2023, 11:13 AM
This revision was automatically updated to reflect the committed changes.