Skip to content

Conversation

MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Mar 1, 2023

This was the only predicate in the use-use dataflow library that was flagged by Schack's regexp. Indeed, it looks like a true positive:

rec SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f(unique numbered_tuple instr) :- 
SENTINEL SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentFwd#1#f
SENTINEL project#SsaInternalsCommon#3c4fa02d::Cached::isSink#2#ff
[[[ BASE CASE ]]]
{1} r1 = JOIN SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentFwd#1#f WITH project#SsaInternalsCommon#3c4fa02d::Cached::isSink#2#ff ON FIRST 1 OUTPUT Lhs.0 'instr'
return r1

[[[ SEMINAIVE VARIANT - STANDARD ORDER]]]
{3} r2 = _SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentFwd#1#f__DataFlowPrivate#fbdd7bd7::getAUse__#loop_invariant_prefix AND NOT SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f#prev(Lhs.2 'instr')
{2} r3 = JOIN r2 WITH DataFlowUtil#47741e1f::conversionFlow#4#ffff_021#join_rhs ON FIRST 2 OUTPUT Rhs.2, Lhs.2 'instr'
{1} r4 = JOIN r3 WITH SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f#prev_delta ON FIRST 1 OUTPUT Lhs.1 'instr'
return r4

[[[ SEMINAIVE VARIANT - ORDER UP TO SIZE 500000]]]
{2} r5 = SCAN SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f#prev_delta OUTPUT In.0, false
{1} r6 = JOIN r5 WITH DataFlowUtil#47741e1f::conversionFlow#4#ffff_120#join_rhs ON FIRST 2 OUTPUT Rhs.2
{1} r7 = JOIN r6 WITH __DataFlowPrivate#fbdd7bd7::getAUse#1#ff_DataFlowPrivate#fbdd7bd7::getAUse#1#ff_011#unique_term#join__#join_rhs ON FIRST 1 OUTPUT Rhs.1 'instr'
{1} r8 = JOIN r7 WITH SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentFwd#1#f ON FIRST 1 OUTPUT Lhs.0 'instr'
{1} r9 = r8 AND NOT SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f#prev(Lhs.0 'instr')
return r9

Notice that in the [[[ SEMINAIVE VARIANT - STANDARD ORDER]]] version of the predicate the #prev predicate is used before joining with #prev_delta (which doesn't happen in the [[[ SEMINAIVE VARIANT - ORDER UP TO SIZE 500000]]] case). This means we keep SCAN'ing all the previous iterations, resulting in unexpected quadratic complexity. After this PR we get:

rec SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f(unique numbered_tuple instr) :- 
    SENTINEL SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentFwd#1#f
    SENTINEL project#SsaInternalsCommon#3c4fa02d::Cached::isSink#2#ff
    [[[ BASE CASE ]]]
    {1} r1 = JOIN SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentFwd#1#f WITH project#SsaInternalsCommon#3c4fa02d::Cached::isSink#2#ff ON FIRST 1 OUTPUT Lhs.0 'instr'
    return r1

    [[[ SEMINAIVE VARIANT - STANDARD ORDER]]]
    {2} r2 = SCAN SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f#prev_delta OUTPUT In.0, false
    {1} r3 = JOIN r2 WITH DataFlowUtil#47741e1f::conversionFlow#4#ffff_120#join_rhs ON FIRST 2 OUTPUT Rhs.2
    {1} r4 = JOIN r3 WITH __DataFlowPrivate#fbdd7bd7::getAUse#1#ff_DataFlowPrivate#fbdd7bd7::getAUse#1#ff_011#unique_term#join__#join_rhs ON FIRST 1 OUTPUT Rhs.1 'instr'
    {1} r5 = JOIN r4 WITH SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentFwd#1#f ON FIRST 1 OUTPUT Lhs.0 'instr'
    {1} r6 = r5 AND NOT SsaInternalsCommon#3c4fa02d::Cached::convertsIntoArgumentRev#1#f#prev(Lhs.0 'instr')
    return r6

which doesn't contain the bad pattern 🎉.

@MathiasVP MathiasVP requested a review from a team as a code owner March 1, 2023 15:22
@github-actions github-actions bot added the C++ label Mar 1, 2023
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Mar 1, 2023
@MathiasVP MathiasVP merged commit c2efb4d into github:mathiasvp/replace-ast-with-ir-use-usedataflow Mar 2, 2023
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