tejohnson added a comment.

Adding Wei who works on SamplePGO on our end currently.



================
Comment at: llvm/lib/Passes/PassBuilder.cpp:665-668
+    // We must also remove exception handling before attaching sample profile
+    // data.
+    MPM.addPass(
+        createModuleToPostOrderCGSCCPassAdaptor(PostOrderFunctionAttrsPass()));
----------------
chandlerc wrote:
> Does the this need to go before the EarlyFPM as well as before the sample 
> profile loader?
> 
> Also, this is ... a pretty expensive thing to do... Do we really need this? 
> We've been using ThinLTO and sample PGO with the new PM for a long time and 
> haven't seen any real issue. I think we can probably just leave this 
> difference between the two pass managers and remove the test for this pass 
> running rather than adding it.
> 
> CC-ing some others just to double check, but I'm not aware of any issues 
> we've seen with PGO that were because of this discrepancy.
We don't tend to use exception handling so might not be hitting the issue 
described in the headline.

If this is fixing an issue with SamplePGO and EH cleanup interactions, can you 
add a test that tests for that (i.e. is there a code optimization issue 
currently?).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63626/new/

https://reviews.llvm.org/D63626



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to