<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 9, 2019 at 8:12 AM David Greene <<a href="mailto:dag@cray.com">dag@cray.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Mehdi AMINI via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> writes:<br>
<br>
>> I have a bit of concern about this sort of thing - worrying it'll lead to<br>
>> people being less cautious about writing the more isolated tests.<br>
>><br>
><br>
> I have the same concern. I really believe we need to be careful about<br>
> testing at the right granularity to keep things both modular and the<br>
> testing maintainable (for instance checking vectorized ASM from a C++<br>
> source through clang has always been considered a bad FileCheck practice).<br>
> (Not saying that there is no space for better integration testing in some<br>
> areas).<br>
<br>
I absolutely disagree about vectorization tests.  We have seen<br>
vectorization loss in clang even though related LLVM lit tests pass,<br>
because something else in the clang pipeline changed that caused the<br>
vectorizer to not do its job.  </blockquote><div><br></div><div>Of course, and as I mentioned I tried to add these tests (probably 4 or 5 years ago), but someone (I think Chandler?) was asking me at the time: does it affect a benchmark performance? If so why isn't it tracked there? And if not does it matter? </div><div>The benchmark was presented as the actual way to check this invariant (because you're only vectoring to get performance, not for the sake of it).</div><div>So I never pursued, even if I'm a bit puzzled that we don't have such tests.<br></div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">We need both kinds of tests.  There are<br>
many asm tests of value beyond vectorization and they should include<br>
component and well as end-to-end tests.<br>
<br>
> For instance I remember asking about implementing test based on checking if<br>
> some loops written in C source file were properly vectorized by the -O2 /<br>
> -O3 pipeline and it was deemed like the kind of test that we don't want to<br>
> maintain: instead I was pointed at the test-suite to add better benchmarks<br>
> there for the end-to-end story. What is interesting is that the test-suite<br>
> is not gonna be part of the monorepo!<br>
<br>
And it shouldn't be.  It's much too big.  But there is a place for small<br>
end-to-end tests that live alongside the code.<br>
<br>
>>> We could, for example, create<br>
>>> a top-level "test" directory and put end-to-end tests there.  Some of<br>
>>> the things that could be tested include:<br>
>>><br>
>>> - Pipeline execution (debug-pass=Executions)<br>
>>><br>
>>> - Optimization warnings/messages<br>
>>> - Specific asm code sequences out of clang (e.g. ensure certain loops<br>
>>>   are vectorized)<br>
>>> - Pragma effects (e.g. ensure loop optimizations are honored)<br>
>>> - Complete end-to-end PGO (generate a profile and re-compile)<br>
>>> - GPU/accelerator offloading<br>
>>> - Debuggability of clang-generated code<br>
>>><br>
>>> Each of these things is tested to some degree within their own<br>
>>> subprojects, but AFAIK there are currently no dedicated tests ensuring<br>
>>> such things work through the entire clang pipeline flow and with other<br>
>>> tools that make use of the results (debuggers, etc.).  It is relatively<br>
>>> easy to break the pipeline while the individual subproject tests<br>
>>> continue to pass.<br>
>>><br>
>><br>
><br>
> I'm not sure I really see much in your list that isn't purely about testing<br>
> clang itself here?<br>
<br>
Debugging and PGO involve other components, no?</blockquote><div><br></div><div>I don't think that you need anything else than LLVM core (which is a dependency of clang) itself?</div><div><br></div><div>Things like PGO (unless you're using frontend instrumentation) don't even have anything to do with clang, so we may get into the situation David mentioned where we would rely on clang to test LLVM features, which I find non-desirable.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">  If we want to put clang<br>
end-to-end tests in the clang subdirectory, that's fine with me.  But we<br>
need a place for tests that cut across components.<br>
<br>
I could also imagine llvm-mca end-to-end tests through clang.<br>
<br>
> Actually the first one seems more of a pure LLVM test.<br>
<br>
Definitely not.  It would test the pipeline as constructed by clang,<br>
which is very different from the default pipeline constructed by<br>
opt/llc. </blockquote><div><br></div><div>I am not convinced it is "very" difference (they are using the PassManagerBuilder AFAIK), I am only aware of very subtle difference.</div><div>But more fundamentally: *should* they be different? I would want `opt -O3` to be able to reproduce 1-1 the clang pipeline.</div><div>Isn't it the role of LLVM PassManagerBuilder to expose what is the "-O3" pipeline?</div><div>If we see the PassManagerBuilder as the abstraction for the pipeline, then I don't see what testing belongs to clang here, this seems like a layering violation (and maintaining the PassManagerBuilder in LLVM I wouldn't want to have to update the tests of all the subproject using it because they retest the same feature).</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> The old and new pass managers also construct different<br>
pipelines.  As we have seen with various mailing list messages, this is<br>
surprising to users.  Best to document and check it with testing.<br></blockquote><div><br></div><div>Yes: both old and new pass managers are LLVM components, so hopefully that are documented and tested in LLVM :)</div><div><br></div><div>-- </div><div>Mehdi</div><div> </div></div></div></div></div>