Skip to content

Fixing Compiler bug#5557

Merged
kfcripps merged 4 commits intop4lang:mainfrom
MathewZach123:test_51_v1
Mar 26, 2026
Merged

Fixing Compiler bug#5557
kfcripps merged 4 commits intop4lang:mainfrom
MathewZach123:test_51_v1

Conversation

@MathewZach123
Copy link
Copy Markdown
Contributor

@MathewZach123 MathewZach123 commented Mar 23, 2026

This PR fixes #5549 caused due to DAG-cached stale references in FunctionsInliner by modifying DoRemoveActionParameters to handle inout parameters separately. A new ExtractArrayIndices Transform extracts side-effecting array indices into temporary variables before generating copy-in/copy-out, so the function call is evaluated once.

Updated IR for Test 1 (testdata/p4_16_samples/inline-function-to-array-index.p4)

   @noWarn("unused") @name(".NoAction") action NoAction_0() {
   }
   @name("hdr") h_t hdr_1;
   bit<3> tmp_idx;
   @name("a") action a_0() {
       tmp_idx = max(3w0, 3w0);
       hdr_1 = h.hdr_arr[tmp_idx];
       h.hdr_arr[tmp_idx] = hdr_1;

   }

Updated IR for Test 2 (testdata/p4_16_samples/inline-function-to-array-index-chained.p4)

    @noWarn("unused") @name(".NoAction") action NoAction_0() {
    }
    @name("val") bit<8> val_1;
    bit<3> tmp_idx;
    bit<3> tmp_idx_0;
    @name("a") action a_0() {
        tmp_idx = max(3w0, 3w0);
        tmp_idx_0 = max(3w0, 3w0);
        val_1 = h.mid_arr[tmp_idx].inner_arr[tmp_idx_0].f2;
        val_1 = 8w1;
        h.mid_arr[tmp_idx].inner_arr[tmp_idx_0].f2 = val_1;
    }
 ``

@MathewZach123 MathewZach123 changed the title [DONOT MERGE] Fixing Compiler bug Mar 23, 2026
@MathewZach123 MathewZach123 requested a review from kfcripps March 23, 2026 21:57
@kfcripps kfcripps added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Mar 23, 2026
ChrisDodd
ChrisDodd previously approved these changes Mar 23, 2026
@ChrisDodd ChrisDodd self-requested a review March 23, 2026 23:11
@ChrisDodd
Copy link
Copy Markdown
Contributor

Actaully, looking at this a bit more, while this fixes this case, I'm not sure this is the correct fix overall -- it is fixing the inlining of the duplicated method call by inlining it twice, but the duplication of the method call might be problematic if the method itself contains side effects.

@ChrisDodd
Copy link
Copy Markdown
Contributor

See the comment I added to #5549

@MathewZach123 MathewZach123 marked this pull request as draft March 24, 2026 15:06
@MathewZach123 MathewZach123 marked this pull request as ready for review March 24, 2026 17:42
@MathewZach123 MathewZach123 requested review from ChrisDodd and removed request for ChrisDodd March 24, 2026 21:08
@MathewZach123
Copy link
Copy Markdown
Contributor Author

Thanks @ChrisDodd

@kfcripps
Copy link
Copy Markdown
Contributor

@MathewZach123 You should be able to remove this line if you rebase: https://github.com/p4lang/p4c/blob/main/backends/p4tools/modules/smith/scripts/compilation-test.sh#L15

Mathew Suresh Zachariah added 3 commits March 25, 2026 20:22
Signed-off-by: Mathew Suresh Zachariah <mazachar@pensando.io>
Signed-off-by: Mathew Suresh Zachariah <mazachar@pensando.io>
Signed-off-by: Mathew Suresh Zachariah <mazachar@pensando.io>
Signed-off-by: Mathew Suresh Zachariah <mazachar@pensando.io>
@kfcripps kfcripps added this pull request to the merge queue Mar 26, 2026
Merged via the queue into p4lang:main with commit 0996c84 Mar 26, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Topics concerning the core segments of the compiler (frontend, midend, parser)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler Bug: Function inlining fails when return value is used as array index in action argument

3 participants