Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Apr 19, 2021

@github-actions github-actions bot added the C# label Apr 19, 2021
@hvitved hvitved force-pushed the csharp/ssa/perf-tweaks branch from 5673998 to 9128ec7 Compare April 19, 2021 13:51
@hvitved hvitved marked this pull request as ready for review April 20, 2021 17:48
@hvitved hvitved requested a review from a team as a code owner April 20, 2021 17:48
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Apr 20, 2021
Copy link
Contributor

@tamasvajk tamasvajk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff job has failed.

predicate defAdjacentRead(Definition def, BasicBlock bb1, BasicBlock bb2, int i2) {
varBlockReaches(def, bb1, bb2) and
ssaRefRank(bb2, i2, def.getSourceVariable(), SsaRead()) = 1 and
variableRead(bb2, i2, _, _)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was variableRead(bb2, i2, _, _) removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is implied by ssaRefRank(bb2, i2, def.getSourceVariable(), SsaRead()), so it was simply redundant.

@hvitved
Copy link
Contributor Author

hvitved commented Apr 21, 2021

The diff job has failed.

Sorry, I forgot to update the description with the new job: https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/1019/

@hvitved hvitved merged commit 7080b25 into github:main Apr 21, 2021
@hvitved hvitved deleted the csharp/ssa/perf-tweaks branch April 21, 2021 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants