[UR] Add adapter leak-checking tests #1029

kswiecicki · 2023-11-02T10:41:57Z

No description provided.

pbalcer

can you add a test?

kswiecicki · 2023-11-02T15:32:01Z

can you add a test?

Done.
Apparently there's a limit to the number of groups in a regex that ctest can compile (it fails as if the match failed) so I've changed (.*) to .* everywhere.

pbalcer · 2023-11-03T11:21:09Z

Done. Apparently there's a limit to the number of groups in a regex that ctest can compile (it fails as if the match failed) so I've changed (.*) to .* everywhere.

Use the python match scripts in the future.

kswiecicki · 2023-11-03T11:26:33Z

Done. Apparently there's a limit to the number of groups in a regex that ctest can compile (it fails as if the match failed) so I've changed (.*) to .* everywhere.

Use the python match scripts in the future.

I've looked into the match.py but it seems to support only line-by-line matching. In my case I needed to match some unknown number of backtrace lines.

kswiecicki · 2023-11-07T11:41:10Z

Done. Apparently there's a limit to the number of groups in a regex that ctest can compile (it fails as if the match failed) so I've changed (.*) to .* everywhere.

Use the python match scripts in the future.

I've added the {{IGNORE}} tag to match.py script and updated leak checking tests.

cmake/match.py

pbalcer · 2023-11-20T11:35:13Z

cmake/match.py


-if len(match_lines) != len(input_lines):
-    sys.exit(f"Line count doesn't match (is: {len(input_lines)}, expected: {len(match_lines)})")
+def print_incorrect_match(match_line, present, expected):


pbalcer · 2023-11-20T11:40:03Z

cmake/match.py

+    match_idx = 0
+    tags_in_effect = []
+    status = Status.PROCESSING
+    while status == Status.PROCESSING:


this doesn't seem necessary, this condition will never be false as far as I can tell. You can just do a while True: loop and check status at the end (and exit appropriately).

I've moved the status variable inside the loop and changed the loop condition.
If I were to move the status check at the end, then I would need to check if either input or match file are empty to avoid index error. Logger has such test where both input and match files are empty.

Also, the match case statement (switch equivalent) is not available in our current supported python version.

pbalcer · 2023-11-20T11:42:42Z

cmake/match.py

+
+        match_line = match_lines[match_idx]
+        if match_line.startswith(Tag.OPT.value):
+            tags_in_effect.append(Tag.OPT)


doesn't seem like there can be more than one active tag?

In the case below, there would be a point when ignore tag (active until the next match) is applied, then the optional tag (active until the next line) is applied next.

{{IGNORE}} {{OPT}} optional line

pbalcer · 2023-11-20T11:50:01Z

I don't think we can merge this is as, it will lead to conflicts with the adapters branch. This patch needs to be rebased on adapters or wait until the branches are merged.

kswiecicki · 2023-11-20T15:43:13Z

I don't think we can merge this is as, it will lead to conflicts with the adapters branch. This patch needs to be rebased on adapters or wait until the branches are merged.

Alright, I can wait for the adapters branch to be merged with main.

codecov-commenter · 2023-12-04T10:20:43Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (42874bc) 15.73% compared to head (8c63208) 15.80%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1029      +/-   ##
==========================================
+ Coverage   15.73%   15.80%   +0.06%     
==========================================
  Files         223      223              
  Lines       31478    31481       +3     
  Branches     3558     3558              
==========================================
+ Hits         4954     4976      +22     
+ Misses      26473    26454      -19     
  Partials       51       51

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pbalcer · 2023-12-11T11:34:02Z

this does not appear to be changing anything but tests?

kswiecicki · 2023-12-11T12:31:34Z

this does not appear to be changing anything but tests?

The adapter refcount fix was merged on the adapters branch (#1150) and is now present on the main.
I've changed the title of this PR.

kbenzie · 2023-12-13T12:53:41Z

Sounds good @kswiecicki! I'm also working on resolving the issue I found in the test fixtures.

Since 5dc6e46 CTS tests which execute kernels have been failing to create programs. This has been hidden due to the match files containing expected failures in combination with an bug in the match file script found while debugging oneapi-src#1029 check failures. This patch adds `uur::KernelsEnvironment::CreateProgram()` which automatically handles the differences between the HIP adapter and others. On the hip path `urProgramCreateWithIL` is not longer called because it is not supported by the adatper and `urProgramCreateWithBinary` is used instead. The non-HIP path continues to use `urProgramCreateWithIL`.

When tests are aborted or failed at the assertion, the match script receives "Aborted" or "Segmentation fault" as an input and compares it with the match file. Previous match script allowed those tests to pass despite not matching this input.

- oneapi-src/unified-runtime#1035 - oneapi-src/unified-runtime#1029 - oneapi-src/unified-runtime#1242

pbalcer reviewed Nov 2, 2023

View reviewed changes

kswiecicki force-pushed the val-refcount-adapter-fix branch from e31605c to 08fee15 Compare November 2, 2023 15:33

pbalcer approved these changes Nov 3, 2023

View reviewed changes

kswiecicki force-pushed the val-refcount-adapter-fix branch from e6b4224 to f7c65e1 Compare November 7, 2023 11:39

PatKamin reviewed Nov 8, 2023

View reviewed changes

cmake/match.py Outdated Show resolved Hide resolved

kswiecicki force-pushed the val-refcount-adapter-fix branch from f7c65e1 to 8c51b46 Compare November 8, 2023 10:01

PatKamin approved these changes Nov 8, 2023

View reviewed changes

kswiecicki force-pushed the val-refcount-adapter-fix branch from 8c51b46 to c235b5c Compare November 8, 2023 11:51

kswiecicki force-pushed the val-refcount-adapter-fix branch from c235b5c to 8f2502d Compare November 20, 2023 09:49

pbalcer reviewed Nov 20, 2023

View reviewed changes

kswiecicki force-pushed the val-refcount-adapter-fix branch from 8f2502d to 338bca3 Compare November 20, 2023 15:27

kswiecicki force-pushed the val-refcount-adapter-fix branch from 338bca3 to 96219d8 Compare November 21, 2023 10:34

kswiecicki mentioned this pull request Nov 29, 2023

[UR] Add lifetime validation to validation layer #1121

Merged

kswiecicki force-pushed the val-refcount-adapter-fix branch from 96219d8 to 2a4d4d6 Compare December 4, 2023 10:03

kswiecicki force-pushed the val-refcount-adapter-fix branch from 2a4d4d6 to da0aab2 Compare December 8, 2023 14:30

kswiecicki requested a review from a team as a code owner December 8, 2023 14:30

kswiecicki force-pushed the val-refcount-adapter-fix branch 3 times, most recently from f63e9a3 to c5d95af Compare December 8, 2023 15:11

kswiecicki force-pushed the val-refcount-adapter-fix branch from 56790be to 591db37 Compare December 11, 2023 11:41

kswiecicki changed the title ~~[UR] Fix adapter refcount tracking~~ [UR] Add adapter leak-checking tests Dec 11, 2023

kbenzie mentioned this pull request Dec 13, 2023

[CTS] Don't use urProgramCreateWithIL on HIP adapter #1184

Merged

kswiecicki force-pushed the val-refcount-adapter-fix branch 5 times, most recently from 1011a06 to b7f9af9 Compare December 20, 2023 14:21

kswiecicki force-pushed the val-refcount-adapter-fix branch 2 times, most recently from 0a702ea to 237c23b Compare December 29, 2023 10:00

kswiecicki force-pushed the val-refcount-adapter-fix branch from 237c23b to 7eb95a3 Compare January 8, 2024 08:03

pbalcer approved these changes Jan 9, 2024

View reviewed changes

pbalcer added the ready to merge Added to PR's which are ready to merge label Jan 9, 2024

kswiecicki force-pushed the val-refcount-adapter-fix branch 2 times, most recently from f6871e1 to 5551790 Compare January 9, 2024 15:32

PatKamin approved these changes Jan 10, 2024

View reviewed changes

kswiecicki force-pushed the val-refcount-adapter-fix branch 2 times, most recently from 88ecbf0 to cc66c29 Compare January 10, 2024 16:12

[UR] Add adapter leak-checking tests

bdf2918

kswiecicki force-pushed the val-refcount-adapter-fix branch from cc66c29 to e734fbc Compare January 10, 2024 16:18

pbalcer mentioned this pull request Jan 10, 2024

[CUDA] Segmentation fault in CTS enqueue tests #1224

Closed

kswiecicki added 4 commits January 11, 2024 11:08

Add ignore tag to the match.py script

d6930c7

Refactor leak checking match tests

2e10197

[OpenCL] Fix suffixes in adapter match tests

8c63208

kswiecicki force-pushed the val-refcount-adapter-fix branch from e734fbc to 8c63208 Compare January 11, 2024 10:08

kbenzie approved these changes Jan 11, 2024

View reviewed changes

kbenzie merged commit e1414e1 into oneapi-src:main Jan 11, 2024
51 checks passed

mfrancepillois mentioned this pull request Jan 12, 2024

[UR] Bump tag to d06ba9d intel/llvm#12381

Merged

aelovikov-intel pushed a commit to intel/llvm that referenced this pull request Jan 12, 2024

[UR] Bump tag to d06ba9d (#12381)

6eac61a

- oneapi-src/unified-runtime#1035 - oneapi-src/unified-runtime#1029 - oneapi-src/unified-runtime#1242

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UR] Add adapter leak-checking tests #1029

[UR] Add adapter leak-checking tests #1029

kswiecicki commented Nov 2, 2023

pbalcer left a comment

kswiecicki commented Nov 2, 2023

pbalcer commented Nov 3, 2023

kswiecicki commented Nov 3, 2023 •

edited

Loading

kswiecicki commented Nov 7, 2023

pbalcer Nov 20, 2023

kswiecicki Nov 20, 2023

pbalcer Nov 20, 2023

kswiecicki Nov 20, 2023 •

edited

Loading

kswiecicki Nov 20, 2023

pbalcer Nov 20, 2023

kswiecicki Nov 20, 2023

pbalcer commented Nov 20, 2023

kswiecicki commented Nov 20, 2023

codecov-commenter commented Dec 4, 2023 •

edited

Loading

pbalcer commented Dec 11, 2023

kswiecicki commented Dec 11, 2023

kbenzie commented Dec 13, 2023

[UR] Add adapter leak-checking tests #1029

[UR] Add adapter leak-checking tests #1029

Conversation

kswiecicki commented Nov 2, 2023

pbalcer left a comment

Choose a reason for hiding this comment

kswiecicki commented Nov 2, 2023

pbalcer commented Nov 3, 2023

kswiecicki commented Nov 3, 2023 • edited Loading

kswiecicki commented Nov 7, 2023

pbalcer Nov 20, 2023

Choose a reason for hiding this comment

kswiecicki Nov 20, 2023

Choose a reason for hiding this comment

pbalcer Nov 20, 2023

Choose a reason for hiding this comment

kswiecicki Nov 20, 2023 • edited Loading

Choose a reason for hiding this comment

kswiecicki Nov 20, 2023

Choose a reason for hiding this comment

pbalcer Nov 20, 2023

Choose a reason for hiding this comment

kswiecicki Nov 20, 2023

Choose a reason for hiding this comment

pbalcer commented Nov 20, 2023

kswiecicki commented Nov 20, 2023

codecov-commenter commented Dec 4, 2023 • edited Loading

Codecov Report

pbalcer commented Dec 11, 2023

kswiecicki commented Dec 11, 2023

kbenzie commented Dec 13, 2023

kswiecicki commented Nov 3, 2023 •

edited

Loading

kswiecicki Nov 20, 2023 •

edited

Loading

codecov-commenter commented Dec 4, 2023 •

edited

Loading