On Wed, Apr 29, 2015 at 10:05 AM, David Blaikie <dblaikie at gmail.com> wrote:> > > On Wed, Apr 29, 2015 at 9:48 AM, Teresa Johnson <tejohnson at google.com> > wrote: >> >> Ok, thanks for the suggestion. I will rework the tests to do that. > > > In case you haven't found it already, %T in the lit syntax gives you a > uniquely named directory for the testActually %T is just the base directory where all the compiler-rt/profile tests are run, not unique per .c test. I fixed the tests by creating a "%t.d" directory and cd'ing into it and generating all output locally in that directory. Patch uploaded here, PTAL: http://reviews.llvm.org/D9349 Thanks, Teresa> >> >> Teresa >> >> On Wed, Apr 29, 2015 at 9:47 AM, Eric Christopher <echristo at gmail.com> >> wrote: >> > >> > >> > On Wed, Apr 29, 2015 at 9:36 AM Justin Bogner <mail at justinbogner.com> >> > wrote: >> >> >> >> Teresa Johnson <tejohnson at google.com> writes: >> >> > On Wed, Apr 29, 2015 at 6:27 AM, Renato Golin >> >> > <renato.golin at linaro.org> >> >> > wrote: >> >> >> On 29 April 2015 at 14:16, Teresa Johnson <tejohnson at google.com> >> >> >> wrote: >> >> >>> Two of the compiler-rt/profile tests I added are checking for >> >> >>> similar >> >> >>> behavior with resetting the filename to the default. I wonder if >> >> >>> they >> >> >>> are running in parallel and clobbering each other since the profile >> >> >>> output names are not unique after the reset. >> >> >> >> >> >> Hi Teresa, >> >> >> >> >> >> This would explain the intermittent failures. Maybe making the names >> >> >> unique would fix the issues, would that be an easy change? >> >> > >> >> > After thinking about it I believe this is what is most likely >> >> > happening, and that these two new tests I added will need to be >> >> > reverted: >> >> > >> >> > >> >> > >> >> > compiler-rt/trunk/test/profile/instrprof-override-filename-then-reset-default.c >> >> > >> >> > >> >> > compiler-rt/trunk/test/profile/instrprof-set-filename-then-reset-default.c >> >> > >> >> > since they aren't writing to unique output names. These are 2 of the >> >> > 4 >> >> > tests added in r236056 (the other two are fine). >> >> > >> >> > They can't be changed to write to a unique name since those tests >> >> > were >> >> > specifically testing that the profile output name gets reset to the >> >> > default when null is passed to the profile filename setting >> >> > interfaces. The other profile tests use unique names. >> >> >> >> Could we have these tests `cd` into a uniquely named directory or >> >> something? Maybe that's more complicated than its worth. >> >> >> > >> > That's what we've done in the past and seems a reasonable idea. >> > >> > -eric >> > >> >> >> >> > Since I don't have write access yet, can someone revert those two >> >> > files for me? >> >> > >> >> > Thanks, >> >> > Teresa >> >> >> >> >> >> cheers, >> >> >> --renato >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-- Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
On Wed, Apr 29, 2015 at 10:55 AM, Teresa Johnson <tejohnson at google.com> wrote:> On Wed, Apr 29, 2015 at 10:05 AM, David Blaikie <dblaikie at gmail.com> > wrote: > > > > > > On Wed, Apr 29, 2015 at 9:48 AM, Teresa Johnson <tejohnson at google.com> > > wrote: > >> > >> Ok, thanks for the suggestion. I will rework the tests to do that. > > > > > > In case you haven't found it already, %T in the lit syntax gives you a > > uniquely named directory for the test > > Actually %T is just the base directory where all the > compiler-rt/profile tests are run, not unique per .c test.Ah, sorry for the misleading advice - thanks for the correction.> I fixed the > tests by creating a "%t.d" directory and cd'ing into it and generating > all output locally in that directory. >A quick "grep -r "RUN.*mkdir" certainly shows a few tests doing quite similar things (most use -p, which I see you've done too). You'll probably also need REQUIRES: shell in this test to ensure it doesn't run in environments that don't have a full bash-like shell available (such as Windows). Check the other tests that use mkdir to see that sort of thing.> > Patch uploaded here, PTAL: > http://reviews.llvm.org/D9349 > > Thanks, > Teresa > > > > >> > >> Teresa > >> > >> On Wed, Apr 29, 2015 at 9:47 AM, Eric Christopher <echristo at gmail.com> > >> wrote: > >> > > >> > > >> > On Wed, Apr 29, 2015 at 9:36 AM Justin Bogner <mail at justinbogner.com> > >> > wrote: > >> >> > >> >> Teresa Johnson <tejohnson at google.com> writes: > >> >> > On Wed, Apr 29, 2015 at 6:27 AM, Renato Golin > >> >> > <renato.golin at linaro.org> > >> >> > wrote: > >> >> >> On 29 April 2015 at 14:16, Teresa Johnson <tejohnson at google.com> > >> >> >> wrote: > >> >> >>> Two of the compiler-rt/profile tests I added are checking for > >> >> >>> similar > >> >> >>> behavior with resetting the filename to the default. I wonder if > >> >> >>> they > >> >> >>> are running in parallel and clobbering each other since the > profile > >> >> >>> output names are not unique after the reset. > >> >> >> > >> >> >> Hi Teresa, > >> >> >> > >> >> >> This would explain the intermittent failures. Maybe making the > names > >> >> >> unique would fix the issues, would that be an easy change? > >> >> > > >> >> > After thinking about it I believe this is what is most likely > >> >> > happening, and that these two new tests I added will need to be > >> >> > reverted: > >> >> > > >> >> > > >> >> > > >> >> > > compiler-rt/trunk/test/profile/instrprof-override-filename-then-reset-default.c > >> >> > > >> >> > > >> >> > > compiler-rt/trunk/test/profile/instrprof-set-filename-then-reset-default.c > >> >> > > >> >> > since they aren't writing to unique output names. These are 2 of > the > >> >> > 4 > >> >> > tests added in r236056 (the other two are fine). > >> >> > > >> >> > They can't be changed to write to a unique name since those tests > >> >> > were > >> >> > specifically testing that the profile output name gets reset to the > >> >> > default when null is passed to the profile filename setting > >> >> > interfaces. The other profile tests use unique names. > >> >> > >> >> Could we have these tests `cd` into a uniquely named directory or > >> >> something? Maybe that's more complicated than its worth. > >> >> > >> > > >> > That's what we've done in the past and seems a reasonable idea. > >> > > >> > -eric > >> > > >> >> > >> >> > Since I don't have write access yet, can someone revert those two > >> >> > files for me? > >> >> > > >> >> > Thanks, > >> >> > Teresa > >> >> >> > >> >> >> cheers, > >> >> >> --renato > >> >> _______________________________________________ > >> >> LLVM Developers mailing list > >> >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> > >> > >> > >> -- > >> Teresa Johnson | Software Engineer | tejohnson at google.com | > 408-460-2413 > >> _______________________________________________ > >> LLVM Developers mailing list > >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > > > > > -- > Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150429/38500115/attachment.html>
On Wed, Apr 29, 2015 at 11:19 AM, David Blaikie <dblaikie at gmail.com> wrote:> > > On Wed, Apr 29, 2015 at 10:55 AM, Teresa Johnson <tejohnson at google.com> > wrote: >> >> On Wed, Apr 29, 2015 at 10:05 AM, David Blaikie <dblaikie at gmail.com> >> wrote: >> > >> > >> > On Wed, Apr 29, 2015 at 9:48 AM, Teresa Johnson <tejohnson at google.com> >> > wrote: >> >> >> >> Ok, thanks for the suggestion. I will rework the tests to do that. >> > >> > >> > In case you haven't found it already, %T in the lit syntax gives you a >> > uniquely named directory for the test >> >> Actually %T is just the base directory where all the >> compiler-rt/profile tests are run, not unique per .c test. > > > Ah, sorry for the misleading advice - thanks for the correction. > >> >> I fixed the >> tests by creating a "%t.d" directory and cd'ing into it and generating >> all output locally in that directory. > > > A quick "grep -r "RUN.*mkdir" certainly shows a few tests doing quite > similar things (most use -p, which I see you've done too). > > You'll probably also need REQUIRES: shell in this test to ensure it doesn't > run in environments that don't have a full bash-like shell available (such > as Windows). Check the other tests that use mkdir to see that sort of thing.Some have the "REQUIRES: shell" (e.g. tools/clang/test/VFS/include-virtual-from-real.c) and some don't (e.g. ./tools/clang/test/Analysis/html-diags.c). I looked around for documentation on when "REQUIRES: shell" is needed but couldn't find anything specific. The latter test (html-diags.c), which doesn't have the REQUIRES also does mkdir -p, cd, rm -rf", which are the same shell-like operations my tests are doing. The former (include-virtual-from-real.c) is doing some additional shell-like operations such as sed and echo, which I am not doing. Is there a list of operations safe to do without adding the "REQUIRES: shell"? Thanks, Teresa> >> >> >> Patch uploaded here, PTAL: >> http://reviews.llvm.org/D9349 >> >> Thanks, >> Teresa >> >> > >> >> >> >> Teresa >> >> >> >> On Wed, Apr 29, 2015 at 9:47 AM, Eric Christopher <echristo at gmail.com> >> >> wrote: >> >> > >> >> > >> >> > On Wed, Apr 29, 2015 at 9:36 AM Justin Bogner <mail at justinbogner.com> >> >> > wrote: >> >> >> >> >> >> Teresa Johnson <tejohnson at google.com> writes: >> >> >> > On Wed, Apr 29, 2015 at 6:27 AM, Renato Golin >> >> >> > <renato.golin at linaro.org> >> >> >> > wrote: >> >> >> >> On 29 April 2015 at 14:16, Teresa Johnson <tejohnson at google.com> >> >> >> >> wrote: >> >> >> >>> Two of the compiler-rt/profile tests I added are checking for >> >> >> >>> similar >> >> >> >>> behavior with resetting the filename to the default. I wonder if >> >> >> >>> they >> >> >> >>> are running in parallel and clobbering each other since the >> >> >> >>> profile >> >> >> >>> output names are not unique after the reset. >> >> >> >> >> >> >> >> Hi Teresa, >> >> >> >> >> >> >> >> This would explain the intermittent failures. Maybe making the >> >> >> >> names >> >> >> >> unique would fix the issues, would that be an easy change? >> >> >> > >> >> >> > After thinking about it I believe this is what is most likely >> >> >> > happening, and that these two new tests I added will need to be >> >> >> > reverted: >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> > compiler-rt/trunk/test/profile/instrprof-override-filename-then-reset-default.c >> >> >> > >> >> >> > >> >> >> > >> >> >> > compiler-rt/trunk/test/profile/instrprof-set-filename-then-reset-default.c >> >> >> > >> >> >> > since they aren't writing to unique output names. These are 2 of >> >> >> > the >> >> >> > 4 >> >> >> > tests added in r236056 (the other two are fine). >> >> >> > >> >> >> > They can't be changed to write to a unique name since those tests >> >> >> > were >> >> >> > specifically testing that the profile output name gets reset to >> >> >> > the >> >> >> > default when null is passed to the profile filename setting >> >> >> > interfaces. The other profile tests use unique names. >> >> >> >> >> >> Could we have these tests `cd` into a uniquely named directory or >> >> >> something? Maybe that's more complicated than its worth. >> >> >> >> >> > >> >> > That's what we've done in the past and seems a reasonable idea. >> >> > >> >> > -eric >> >> > >> >> >> >> >> >> > Since I don't have write access yet, can someone revert those two >> >> >> > files for me? >> >> >> > >> >> >> > Thanks, >> >> >> > Teresa >> >> >> >> >> >> >> >> cheers, >> >> >> >> --renato >> >> >> _______________________________________________ >> >> >> LLVM Developers mailing list >> >> >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> >> >> >> >> -- >> >> Teresa Johnson | Software Engineer | tejohnson at google.com | >> >> 408-460-2413 >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >> > >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413 > >-- Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413