leonardchan added a comment. >> **CodeGen/pgo-sample.c [No new fix]**: The test checks that `PruneEH` runs, >> but there doesn’t seem to be a corresponding new PM pass for it. Should >> there be? If there is one then we can just check for that in the debug PM >> dump. > > The analog would be `function-attrs` and `function(simplify-cfg)` in some > combination. Maybe this should just check that some specific thing is > optimized away at `-O2` rather than checking the specific details of pass > pipeline construction?
For now I added a check that those 2 extra passes are run in the debug pass dump. It seems that the original test just wanted to see that a particular pass was run, so I left it at that, but I can still add a separate test/run for checking that `function-attrs` and `function(simplify-cfg)` replace `invoke` instructions at `-O2` (which is what I think PruneEH does). > Yeah, I think the entry/exit pass really wants to come *before* any other > pass. That should be possible even in the new PM by using an extension point? Moved this into D63577 <https://reviews.llvm.org/D63577>. >> **CodeGenCXX/conditional-temporaries.cpp**: The new pm seems to be unable to >> statically determine the return value of some functions. For now I just add >> separate checks for the new PM IR. > > Interesting. Definitely the right thing for this patch. Maybe file a PR to > track understanding why the new PM struggles here. > >> **CodeGenCXX/member-function-pointer-calls.cpp**: Same as >> `CodeGenCXX/conditional-temporaries.cpp`. In this case, the new PM codegen >> also contains calls to lifetime start/end intrinsics. Perhaps this time it >> could be due to the intrinsics not being optimized out? > > Weird, but that's also my best guess. Again, maybe we need a bug to track why > this is different. Created https://bugs.llvm.org/show_bug.cgi?id=42333 to track these 2. >> **CodeGenObjC/os_log.m**: Legacy test expects argument of `ptrtoint` and >> some functions to be a specific type and argument, though the new PM codegen >> uses a different type and argument which are also valid since >> `@llvm.objc.retainAutoreleasedReturnValue(val)` always returns `val`. >> >> **CodeGenObjCXX/os_log.mm**: A function seems to have been inlined under the >> new PM. Here we just prevent inlining for the new PM run since the test >> expects this specific function call. > > You could also use a `noinline` attribute in the code to express the > *specific* call that needs to be retained? Hmm `noinline` doesn't seem to be working in this case. The call is made to a code generated builtin function (`__os_log_helper`). ================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:1135-1139 + PB.registerOptimizerLastEPCallback( + [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) { + FPM.addPass(EntryExitInstrumenterPass(/*PostInlining=*/false)); + }); + ---------------- chandlerc wrote: > FYI, feel free to split out the entry/exit change (and its test) to a > separate patch if you want. Either way really. Separated this into D63577 ================ Comment at: llvm/lib/Passes/PassBuilder.cpp:815-818 + // running remaining passes on the eliminated functions. These should be + // preserved during prelinking for link-time inlining decisions. + if (!LTOPreLink) + MPM.addPass(EliminateAvailableExternallyPass()); ---------------- chandlerc wrote: > I'd suggest splitting this into a separate patch to LLVM and adding an > LLVM-side test to cover it as well. Separated this into D63580 and added an LLVM test in that one Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63156/new/ https://reviews.llvm.org/D63156 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
