<div dir="ltr">Yes you're correct, `-passes=func1,func2` is more correct than `-passes=function(func1),function(func2)` if we want to preserve the exact legacy PM interleaving order.<div><br></div><div>For just function passes it shouldn't matter too much. For interleaved cgscc and function passes it makes more of a difference.</div><div><br></div><div>Perhaps we manually update RUN lines with multiple passes that don't run on the same IR unit as long as there aren't too many. And for the rest automatically use -passes=pass1,pass2.</div><div><br></div><div>When I was making all the tests work with the new PM, there were a couple cases where I had to add -enable-new-pm=0 to the existing RUN line and add a new RUN line with the proper nesting. </div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jul 5, 2021 at 4:09 PM Björn Pettersson A <<a href="mailto:bjorn.a.pettersson@ericsson.com" target="_blank">bjorn.a.pettersson@ericsson.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> -----Original Message-----<br>
> From: llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>> On Behalf Of Arthur<br>
> Eubanks via llvm-dev<br>
> Sent: den 29 juni 2021 03:34<br>
> To: llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
> Subject: [llvm-dev] [RFC][NewPM] Migrating tests from `opt -foo` to `opt -<br>
> passes=foo`<br>
> <br>
> Now that the new PM has been the default for the optimization pipeline for<br>
> a while, I'd like to start thinking about next steps. Not quite deprecating<br>
> the legacy PM for the optimization pipeline yet, since we should wait for<br>
> the next LLVM release. But one thing we can start doing is cleaning up lit<br>
> tests that use opt.<br>
> <br>
> There's been confusion over exactly how the -passes= parsing works, so I've<br>
> updated <a href="https://llvm.org/docs/NewPassManager.html" rel="noreferrer" target="_blank">https://llvm.org/docs/NewPassManager.html</a> (or rather just the<br>
> sources, seems like the webpage hasn't updated yet) with some details.<br>
> <br>
> For background, `opt -foo` is currently already translated to `opt -<br>
> passes=foo` when the new PM is on (which is true by default).<br>
> <br>
> I imagine this would be a short script that has a list of passes from<br>
> PassRegistry.def and the IR unit they operate on, and looks through RUN<br>
> lines with opt, deleting existing pass arguments and replacing them with a<br>
> -passes= argument. If we have more than one pass, each pass would be<br>
> wrapped in the proper adaptor.  For example, `opt -instcombine -globaldce`<br>
> becomes `opt -passes='function(instcombine),globaldce'`.<br>
<br>
For single pass tests I guess this will be quite simple. But isn't it a<br>
bit more complicated for tests running multiple passes?<br>
<br>
If I understand correctly these are kind of equivalent to each other:<br>
<br>
   opt -enable-new-pm=0 -function-pass-1 -function-pass-2<br>
   opt -passes='function-pass-1,function-pass-2'<br>
   opt -passes='function(function-pass-1,function-pass-2)'<br>
<br>
and these are also equivalent to each other<br>
<br>
   opt -enable-new-pm=1 -function-pass-1 -function-pass-2<br>
   opt -passes='function(function-pass-1),function(function-pass-2)'<br>
<br>
However, there are at least some subtle differences between the two<br>
groups. Although I'm not sure exactly if that is a big problem. I have<br>
some vague memories of some test case where it made a difference, but<br>
I don't remember exactly what test it was.<br>
<br>
Perhaps it might impact preservation/caching of analyses passes if going<br>
in/out of pass managers or changing the order in which passes are applied<br>
like that ("pass-1 on all functions and then pass-2 on all functions"<br>
vs "all passes on function-1 and then all passes on function-2")?<br>
<br>
I got a feeling that most tests involving more than one pass, and that<br>
are regression testing some bugfix, are using multiple passes for a reason.<br>
And then it might be a bit more difficult than using a script to rewrite<br>
RUN lines if one wants to make sure that the test case still is valid<br>
in the sense of reproducing/validating the original problem.<br>
<br>
With that said. It might be that the current flip of making -enable-new-pm=1<br>
the default messed up some tests (given that the examples involving<br>
-enable-new-pm=0 and -enable-new-pm=1 above are in different groups). In<br>
some sense we might have been "saved" by still having bots running with<br>
the legacy PM. I do think it often would be more correct to replace<br>
  opt -function-pass-1 -function-pass-2<br>
with<br>
  opt -passes='function-pass-1,function-pass-2'<br>
rather than<br>
  opt -passes='function(function-pass-1),function(function-pass-2)'<br>
<br>
> <br>
> We need to make sure that we don't end up causing too many duplicate RUN<br>
> lines if we have tests that specify both `opt -foo` and `opt -passes=foo`.<br>
> <br>
> We should wait until the next LLVM release before changing all opt tests<br>
> since we do want any bots using the legacy PM to still run opt tests<br>
> against the legacy PM. But until then we can pick a couple lesser-used<br>
> passes/test directories to touch up.<br>
> <br>
> Note that this does not include any opt tests that test passes on available<br>
> under the legacy PM, which would be IR passes in the codegen pipeline.<br>
</blockquote></div>