C++: IR: Fix performance of value-init ranges #177
Conversation
On a snapshot of Postgres, evaluation of `getNextExplicitlyInitializedElementAfter#fff#antijoin_rhs#1` took forever, preventing the computation of the IR. I haven't been able to reproduce it with a small test case, but the implementation of `getNextExplicitlyInitializedElementAfter` was fragile because it called the inline predicate `ArrayAggregateLiteral.isInitialized`. It also seemed inefficient that `getNextExplicitlyInitializedElementAfter` was computed for many values of its parameters that were never needed by the caller. This commit replaces `getNextExplicitlyInitializedElementAfter` with a new predicate named `getEndOfValueInitializedRange`, which should have the same behavior but a more efficient implementation. It uses a helper predicate `getNextExplicitlyInitializedElementAfter`, which shares its name with the now-deleted predicate but has behavior that I think matches the name.
| isFirstValueInitializedElementInRange(initList, elementIndex) and | ||
| elementCount = | ||
| getNextExplicitlyInitializedElementAfter(initList, elementIndex) - | ||
| getEndOfValueInitializedRange(initList, elementIndex) - |
There was a problem hiding this comment.
Don't you need an or getNextExplicitlyInitializedElementAfter too?
| private int getEndOfValueInitializedRange(ArrayAggregateLiteral initList, int afterElementIndex) { | ||
| result = getNextExplicitlyInitializedElementAfter(initList, afterElementIndex) | ||
| or | ||
| isFirstValueInitializedElementInRange(initList, afterElementIndex) and |
There was a problem hiding this comment.
Shouldn't this be in both halves of the disjunction?
There was a problem hiding this comment.
getNextExplicitlyInitializedElementAfter calls isFirstValueInitializedElementInRange, so adding another call in this predicate would be redundant.
|
It sounds like I ought to add a test that depends on a non-empty |
|
I've added a simple test to exercise the changed predicate. The results are the same before and after this change. The generated IR looks wrong in both cases, though. All the right code is generated, but Block 0 jumps to Block 2 and never lets Block 1 and Block 3 run. Can you explain this, @dave-bartolomeo? |
|
Bug filed: https://jira.semmle.com/browse/CPP-260 |
Adjust PrintAST query to handle kotlin constructs
On a snapshot of Postgres, evaluation of
getNextExplicitlyInitializedElementAfter#fff#antijoin_rhs#1took forever, preventing the computation of the IR. I haven't been able to reproduce it with a small test case, but the implementation ofgetNextExplicitlyInitializedElementAfterwas fragile because it called the inline predicateArrayAggregateLiteral.isInitialized. It also seemed inefficient thatgetNextExplicitlyInitializedElementAfterwas computed for many values of its parameters that were never needed by the caller.This commit replaces
getNextExplicitlyInitializedElementAfterwith a new predicate namedgetEndOfValueInitializedRange, which should have the same behavior but a more efficient implementation. It uses a helper predicategetNextExplicitlyInitializedElementAfter, which shares its name with the now-deleted predicate but has behavior that I think matches the name.I've made this PR against the RC because I worry that the IR will be useless on too many snapshots without this fix.