Fix PGO consistency asserts with Then only QMarks with likelyhood in AO#129594
Fix PGO consistency asserts with Then only QMarks with likelyhood in AO#129594MichalPetryka wants to merge 1 commit into
Conversation
Removed explicit Then handling in favour of canonicalizing to else first which fixes the issue
github-actions
Bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
Bot
added
the
community-contribution
|
@MihuBot -nuget -jitutils-repo EgorBo/jitutils -jitutils-branch pmi-deterministic-cctors |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
No diffs, less code and fixed an assert. |
| hasTrueExpr = false; | ||
| hasFalseExpr = true; | ||
| } | ||
| assert(hasFalseExpr); // we should always have false now |
There was a problem hiding this comment.
Seems a bit odd to canonicalize to else, is this just to avoid re-reversing the cond?
There was a problem hiding this comment.
Well the code was already reversing stuff which I assume was the part causing issues and doing it properly here once lets us not worry about having to handle only-then and only-else separately.
There was a problem hiding this comment.
I get that, but the existing code that reverses has a TODO saying reversing isn't needed and now you've introduced more reversing.
So I wonder if there is a reversing-free approach that would apply nicely to both.
2 participants
Removed explicit Then handling in favour of canonicalizing to else first which fixes the issue.
cc @AndyAyersMS this seems to solve the issues we discussed on discord