Skip to content

C++: Small simplification#21437

Merged
igfoo merged 1 commit intogithub:mainfrom
igfoo:igfoo/onemk
Mar 10, 2026
Merged

C++: Small simplification#21437
igfoo merged 1 commit intogithub:mainfrom
igfoo:igfoo/onemk

Conversation

@igfoo
Copy link
Member

@igfoo igfoo commented Mar 9, 2026

Removes an unnecessary intermediate variable

@igfoo igfoo added the no-change-note-required This PR does not need a change note label Mar 9, 2026
@github-actions github-actions bot added the C++ label Mar 9, 2026
@igfoo igfoo marked this pull request as ready for review March 9, 2026 18:28
@igfoo igfoo requested a review from a team as a code owner March 9, 2026 18:28
Copilot AI review requested due to automatic review settings March 9, 2026 18:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies the implementation of a helper predicate in the C++ control-flow guard reasoning library by removing an exists wrapper and directly delegating to compares_lt with the transformed offset.

Changes:

  • Simplify compares_ge to a direct call to compares_lt(..., 1 - k, ...).

@jketema
Copy link
Contributor

jketema commented Mar 9, 2026

Could you run some DCA on this?

@jketema
Copy link
Contributor

jketema commented Mar 10, 2026

I'm not sure if that DCA run is a fluke, or whether it points to a join-order problem with this change, but there's a significant slowdown on Wireshark (with a very large standard deviation).

@igfoo
Copy link
Member Author

igfoo commented Mar 10, 2026

That run had

Analysis time, per source
source                  a_m     a_std   b_m     b_std   diff    relative
wireshark__wireshark    1445    13.89   1656    176     211     0.146

The second run has

source                  a_m     a_std   b_m     b_std   diff    relative
wireshark__wireshark    1462    0       1429    0       -33     -0.0226

so I think it was a fluke. I would be surprised if the compiled code was different, anyway.

Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

LGTM

@igfoo igfoo merged commit 341059d into github:main Mar 10, 2026
28 checks passed
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.

3 participants