This is an archive of the discontinued LLVM Phabricator instance.

[test][Inliner] Split inline-cost test files
AbandonedPublic

Authored by aeubanks on Oct 16 2020, 11:19 AM.

Details

Summary

The NPM CGSCCPassManager does not operate on the SCCs in the same order
as the legacy PM. These tests assume that the SCCs would be processed in
the order they are declared in the file, but the NPM processes them
backwards.

So split each test via split-file and test each one individually. This
makes these tests pass under the NPM.

Diff Detail

Unit TestsFailed

Event Timeline

aeubanks created this revision.Oct 16 2020, 11:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 16 2020, 11:19 AM
aeubanks requested review of this revision.Oct 16 2020, 11:19 AM

Ping.
Would it be better to just split the test into multiple files instad of using split-file?

MaskRay added a comment.EditedOct 31 2020, 11:49 PM

Ping.
Would it be better to just split the test into multiple files instad of using split-file?

Apologize for the slow response. I think split-file is better than multiple files in this case, but I want to understand the deep issue here. It seems that ModuleToPostOrderCGSCCPassAdaptor has a reversed processing order (as opposed to the source order) when the functions are independent. This seems unfortunate as this can make all such kind of tests difficult to write.

  • legacy PM: lib/Analysis/CallGraphSCCPass.cpp:541 SCCs are processed in order
  • new PM: include/llvm/Analysis/CGSCCPassManager.h:907 SCCs are processed in the reversed order

I am working on this patch:

--- a/llvm/lib/Analysis/LazyCallGraph.cpp
+++ b/llvm/lib/Analysis/LazyCallGraph.cpp
@@ -1731,7 +1731,7 @@ void LazyCallGraph::buildRefSCCs() {
 
   // The roots will be popped of a stack, so use reverse to get a less
   // surprising order. This doesn't change any of the semantics anywhere.
-  std::reverse(Roots.begin(), Roots.end());
+  //std::reverse(Roots.begin(), Roots.end());
 
   buildGenericSCCs(
       Roots,

D90566 can address the issue without touching ext.ll files.

aeubanks abandoned this revision.Nov 5 2020, 12:37 PM