nikic added a comment.

I'm not a fan of the PGO conditional behavior here. I'd prefer to disable 
speculation unconditionally if that is feasible. This is basically what D153392 
<https://reviews.llvm.org/D153392> did. While the specific test case there was 
addressed in a different way, if we take it in conjunction with this one, I 
think we have enough motivation to do the change.

Another piece of motivation would be that the EarlyFPM change would allow us to 
move LowerExpectIntrinsic to the end of EarlyFPM again, which would make expect 
intrinsics work in Rust again. The pass was moved earlier to make the first 
SimplifyCFG run take them into account, but that would no longer be a problem 
if we disable speculation.

And another piece of motivation would be that we could move SimplifyCFG after 
SROA, where it can be much more effective (but doing so with speculation breaks 
IPSCCP too much). I think doing that needs some further changes, but it's a 
move in the right direction.

So I think the EarlyFPM change at least seems like something we should pretty 
clearly do independently of PGO. The addPGOInstrPasses() change is of course 
PGO specific anyway. I think only the GlobalCleanupPM change may not be 
completely clear cut.



================
Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:1
+;; Check that SimplifyCFG does not attempt speculation until after PGO is
+;; annotated in the IR, and then does not perform it when unprofitable.
----------------
aeubanks wrote:
> hmm typically these phase ordering tests use 
> `llvm/utils/update_test_checks.py`, but that doesn't support the 
> llvm-profdata RUN line. I think a non-update_test_checks test is probably 
> fine for this, @nikic does that make sense?
> 
> 
I thought UTC supports unrecognized commands (by just executing them)... If 
not, a non-UTC test is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155997

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

Reply via email to