Use hip-python API instead of rocm_agent_enumerator #1762

dorde-antic · 2025-03-04T13:57:52Z

Handle agent detection correctly by using hip-python API instead of rocm_agent_enumerator.
Add requirements.txt for external python libraries.

Resolves #1760

mirza-halilcevic · 2025-03-04T14:22:26Z

mlir/test/common_utils/common.py

-            agents = set(x.decode("utf-8") for x in q.stdout.split())
-        return set(a for a in agents if a != "gfx000")
+        agents = set()
+        _, device_count = hip.hipGetDeviceCount()


Consider doing some basic error checking for the HIP calls.

dhernandez0 · 2025-03-04T16:01:04Z

requirements.txt

+PyYAML>=5.4.0, <=6.0.1
+ml_dtypes>=0.1.0, <=0.5.0   # provides several NumPy dtype extensions, including the bf16
+hip-python
+scipy


new line at the end

dhernandez0 · 2025-03-04T16:02:17Z

mlir/test/e2e/generateE2ETest.py

-        agents = set(x.decode("utf-8") for x in q.stdout.split() if x != b"gfx000")
+    agents = set()
+    _, device_count = hip.hipGetDeviceCount()
+    for device in range(device_count):


is it possible to call the function in common.py? same for other files

For now, in commit Use hip-python API fixes, as we talked we have duplicating where files are not in the same directory so that we don't use relative paths until we decide to handle it other way, and I've fixed where it could be done asap.

Maybe, for the future, we should consider having a package for a group of functions we need in different child-directories, so that we can use absolute import anywhere.

I understand we can't import mlir/test/common_utils/common.py in perfRunner.py because it's in utils/performance. But why can't we import common.py here? it's also in mlir/test

We can import it, but in that case we would need to have a package, since those files are not all in same directory (others are in mlir/test/e2e and mlir/test/fusion/e2e). I thought that maybe using package wouldn't be appropriate solution for a file with 2 methods in it.

it's fine then

dhernandez0 · 2025-03-04T16:30:42Z

mlir/test/common_utils/common.py

+            props = hip.hipDeviceProp_t()
+            hip.hipGetDeviceProperties(props,device)
+            agent = props.gcnArchName.decode('utf-8')
+            agents.add(agent) if agent != "gfx000" else None


does this still happen with the API? do we get gfx000?

It didn't notice that this happens when using API, but I have kept it since I saw that it was happening in the previous version of function. Should I remove this check if agent != "gfx000" else None ?

if it's not needed, let's remove it.

dhernandez0 · 2025-03-04T16:33:33Z

mlir/utils/performance/perfRunner.py

+        agent = props.gcnArchName.decode('utf-8')
+
+        if agent in xdlop_supported_gpus:
+            return True


also add this function to wherever we put getArchs()

umangyadav · 2025-03-04T18:47:55Z

mlir/test/common_utils/common.py

@@ -1,5 +1,7 @@
 import os
 import subprocess
+from hip import hip
+from hip import hiprtc


I don't find hiprtc being used. Is it required to be imported ?

I added it when following the hip-python usage instructions, but seems that it is not required for our case. I will remove it and check locally if methods where we use it work without hiprtc.

umangyadav · 2025-03-04T18:49:58Z

requirements.txt

+tomli==2.0.1
+nanobind>=2.4, <3.0
+numpy>=1.19.5, <=2.1.2
+pybind11>=2.10.0, <=2.13.6
+PyYAML>=5.4.0, <=6.0.1
+ml_dtypes>=0.1.0, <=0.5.0   # provides several NumPy dtype extensions, including the bf16


Why do you need all these packages for this PR ?

I suggested to add requirements.txt in this PR to have a list of python dependencies. I think he listed all python libraries we use? @dorde-antic are these used by used or did you add all dependencies from external/llvm-project?

I listed packages from both with versions same as in external/llvm-project. Should I remove those that are only in external, but keep others from the project?
Or I should just keep hip-python which is required for this PR?

I think we can keep all libraries that are actually used for rocmlir (not what external uses). But keep the versions consistent with external.

dhernandez0 · 2025-03-05T12:49:10Z

mlir/test/common_utils/common.py

+
+def is_xdlops_present() -> bool:
+    """This function checks whether a GPU with xdlops support is present"""
+    xdlop_supported_gpus = ['gfx908', 'gfx90a', 'gfx942', 'gfx950']


change to .startsWith("gfx9")

dhernandez0 · 2025-03-05T12:49:32Z

mlir/test/common_utils/common.py

+    """This function checks whether a GPU with xdlops support is present"""
+    xdlop_supported_gpus = ['gfx908', 'gfx90a', 'gfx942', 'gfx950']
+    err_code, device_count = hip.hipGetDeviceCount()
+    if err_code != hip.hipSuccess:


can we reuse get_agents() ? then iterate over the results

dhernandez0 · 2025-03-05T12:50:18Z

mlir/utils/performance/perfRunner.py

@@ -18,6 +18,8 @@
 from typing import Optional, Dict, Tuple
 import numpy as np
 import pandas as pd
+from hip import hip
+from hip import hiprtc


no need to import hiprtc

dhernandez0 · 2025-03-05T12:50:36Z

mlir/utils/performance/perfRunner.py

+        props = hip.hipDeviceProp_t()
+        hip.hipGetDeviceProperties(props,device)
+        agent = props.gcnArchName.decode('utf-8')
+        agents.add(agent) if agent != "gfx000" else None


gfx000 check not needed

dhernandez0 · 2025-03-05T12:52:22Z

mlir/test/e2e/generateE2ETest.py

-        agents = set(x.decode("utf-8") for x in q.stdout.split() if x != b"gfx000")
+    agents = set()
+    _, device_count = hip.hipGetDeviceCount()
+    for device in range(device_count):


I understand we can't import mlir/test/common_utils/common.py in perfRunner.py because it's in utils/performance. But why can't we import common.py here? it's also in mlir/test

dhernandez0 · 2025-03-07T12:33:17Z

mlir/utils/jenkins/Dockerfile

-    && pip3 install -r requirements.txt
+    && pip3 install -r llvm-requirements.txt \
+    && pip3 install -r ../../../requirements.txt \
+    && pip3 install -i https://test.pypi.org/simple hip-python


I think it might work if you add --extra-index-url https://pypi.org/simple to the first line of requirements.txt ?

dhernandez0 · 2025-03-07T12:35:38Z

mlir/utils/jenkins/Dockerfile

@@ -90,7 +90,9 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
    && apt upgrade -y \
    && python3 -m pip install --upgrade pip \
    && pip3 install pandas numpy scipy jinja2 tomli \
-    && pip3 install -r requirements.txt
+    && pip3 install -r llvm-requirements.txt \
+    && pip3 install -r ../../../requirements.txt \


let's use COPY instead

Use hip-python API instead of rocm_agent_enumerator

545c338

dorde-antic requested review from dhernandez0, umangyadav, stefankoncarevic, djramic and mirza-halilcevic March 4, 2025 13:57

dorde-antic marked this pull request as ready for review March 4, 2025 13:58

dorde-antic requested a review from causten as a code owner March 4, 2025 13:58

mirza-halilcevic reviewed Mar 4, 2025

View reviewed changes

dorde-antic marked this pull request as draft March 4, 2025 14:45

dorde-antic marked this pull request as ready for review March 4, 2025 14:45

dhernandez0 reviewed Mar 4, 2025

View reviewed changes

umangyadav reviewed Mar 4, 2025

View reviewed changes

dorde-antic added 2 commits March 5, 2025 07:26

Use hip-python API fixes

f1dd5c4

Use hip-python API instead of rocm_agent_enumerator fixes

ecd3b95

dorde-antic requested review from dhernandez0, mirza-halilcevic and umangyadav March 5, 2025 12:42

dhernandez0 reviewed Mar 5, 2025

View reviewed changes

Reuse get_agents() in is_xdlops_present() and fix perfRunner.py

9e64f46

dorde-antic requested a review from dhernandez0 March 5, 2025 14:19

dhernandez0 approved these changes Mar 5, 2025

View reviewed changes

umangyadav approved these changes Mar 5, 2025

View reviewed changes

dorde-antic added 2 commits March 7, 2025 04:56

Edit requirements in jenkins Dockerfile

7068ce6

Change in Dockerfile for hip-python

92dbbb2

dhernandez0 reviewed Mar 7, 2025

View reviewed changes

danielsu-amd mentioned this pull request Mar 7, 2025

Ex CI: fixes for rocMLIR, rocPyDecode, RVS, rocprof-compute ROCm/ROCm#4462

Merged

dorde-antic added 2 commits March 7, 2025 17:24

Edit requirements.txt

f306c22

Requirements file change

8141d61

dorde-antic force-pushed the issue1760 branch from 6934e71 to 8141d61 Compare March 10, 2025 21:31

dorde-antic added 2 commits March 11, 2025 09:41

Change how hip-python is installed

c383957

Add --no-cache-dir for hip-python installation

fe13347

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use hip-python API instead of rocm_agent_enumerator #1762

Use hip-python API instead of rocm_agent_enumerator #1762

dorde-antic commented Mar 4, 2025 •

edited

Loading

mirza-halilcevic Mar 4, 2025

dhernandez0 Mar 4, 2025

dhernandez0 Mar 4, 2025

dorde-antic Mar 5, 2025

dhernandez0 Mar 5, 2025

dorde-antic Mar 5, 2025 •

edited

Loading

dhernandez0 Mar 5, 2025

dhernandez0 Mar 4, 2025

dorde-antic Mar 5, 2025

dhernandez0 Mar 5, 2025

dhernandez0 Mar 4, 2025

umangyadav Mar 4, 2025

dorde-antic Mar 5, 2025

umangyadav Mar 4, 2025

dhernandez0 Mar 5, 2025

dorde-antic Mar 5, 2025

dhernandez0 Mar 5, 2025

dhernandez0 Mar 5, 2025

dhernandez0 Mar 5, 2025

dhernandez0 Mar 5, 2025

dhernandez0 Mar 5, 2025

dhernandez0 Mar 5, 2025

dhernandez0 Mar 7, 2025

dhernandez0 Mar 7, 2025 •

edited

Loading

Use hip-python API instead of rocm_agent_enumerator #1762

Are you sure you want to change the base?

Use hip-python API instead of rocm_agent_enumerator #1762

Conversation

dorde-antic commented Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dorde-antic Mar 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhernandez0 Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

dorde-antic commented Mar 4, 2025 •

edited

Loading

dorde-antic Mar 5, 2025 •

edited

Loading

dhernandez0 Mar 7, 2025 •

edited

Loading