nod-ai / iree-amd-aie

IREE plugin repository for the AMD AIE accelerator
Apache License 2.0
69 stars 30 forks source link

`iree` submodule #733

Closed makslevental closed 2 months ago

makslevental commented 2 months ago

This PR checks in iree as a submodule instead of using sync_deps.py. I don't know what the original theory of sync_deps.py but this simplifies dev and deploy.

Abhishek-Varma commented 2 months ago

LGTM, but I also don't know why we went for the script in the first place. Maybe @Abhishek-Varma knows?

I don't know but can only speculate based on the doc comment :-

 Casual developers and CI bots invoke this to do the most efficient checkout of dependencies.

Perhaps @stellaraccident can chime in here.

makslevental commented 2 months ago

Casual developers and CI bots invoke this to do the most efficient checkout of dependencies.

Sure but I think no one was/is taking advantage of that. Eg I added skipping torch-mlir but only last week and for that, a simpler way is for iree to make torch-mlir opt-in https://github.com/iree-org/iree/issues/18409 .

makslevental commented 2 months ago

@newling @jtuyls @Abhishek-Varma can I get a re-review. I changed the clone commands/instructions in order to save people time/space (basically there's a way to do what sync_deps was meant to do using just git).

makslevental commented 2 months ago

I was looking at the log and I think the --depth 1 flag is not working:

Run git \
  git \
    -c submodule."third_party/torch-mlir".update=none \
    -c submodule."third_party/stablehlo".update=none \
    -c submodule."src/runtime_src/core/common/aiebu".update=none \
    clone \
    --recursive \
    --shallow-submodules \
    --depth 1 \
    -b $BRANCH_NAME $REPO_ADDRESS .
  shell: /usr/bin/bash -e {0}
  env:
    CACHE_DIR: /_work/iree-amd-aie/iree-amd-aie/.container-cache
    CACHE_KEY: linux-build-test-cpp-asserts-manylinux-v[2](https://github.com/nod-ai/iree-amd-aie/actions/runs/10671927949/job/29578539280?pr=733#step:4:2)-733/merge-405
    BRANCH_NAME: makslevental/iree-submodule
    REPO_ADDRESS: https://github.com/nod-ai/iree-amd-aie
Cloning into '.'...
Submodule 'third_party/XRT' (https://github.com/nod-ai/XRT.git) registered for path 'third_party/XRT'
Submodule 'third_party/aie-rt' (https://github.com/Xilinx/aie-rt.git) registered for path 'third_party/aie-rt'
Submodule 'third_party/bootgen' (https://github.com/Xilinx/bootgen.git) registered for path 'third_party/bootgen'
Submodule 'third_party/iree' (https://github.com/iree-org/iree.git) registered for path 'third_party/iree'
Submodule 'third_party/mlir-air' (https://github.com/nod-ai/mlir-air.git) registered for path 'third_party/mlir-air'
Submodule 'third_party/openssl' (https://github.com/viaduck/openssl-cmake.git) registered for path 'third_party/openssl'
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/XRT'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/aie-rt'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/bootgen'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/iree'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/mlir-air'...
warning: --depth is ignored in local clones; use file:// instead.
done.

(https://github.com/nod-ai/iree-amd-aie/actions/runs/10671927949/job/29578539280?pr=733)

Ya I saw that and that's why I added -shallow-submodules. I'm not 100% on whether this works correctly but 🤷‍♂️

jtuyls commented 2 months ago

I was looking at the log and I think the --depth 1 flag is not working:

Run git \
  git \
    -c submodule."third_party/torch-mlir".update=none \
    -c submodule."third_party/stablehlo".update=none \
    -c submodule."src/runtime_src/core/common/aiebu".update=none \
    clone \
    --recursive \
    --shallow-submodules \
    --depth 1 \
    -b $BRANCH_NAME $REPO_ADDRESS .
  shell: /usr/bin/bash -e {0}
  env:
    CACHE_DIR: /_work/iree-amd-aie/iree-amd-aie/.container-cache
    CACHE_KEY: linux-build-test-cpp-asserts-manylinux-v[2](https://github.com/nod-ai/iree-amd-aie/actions/runs/10671927949/job/29578539280?pr=733#step:4:2)-733/merge-405
    BRANCH_NAME: makslevental/iree-submodule
    REPO_ADDRESS: https://github.com/nod-ai/iree-amd-aie
Cloning into '.'...
Submodule 'third_party/XRT' (https://github.com/nod-ai/XRT.git) registered for path 'third_party/XRT'
Submodule 'third_party/aie-rt' (https://github.com/Xilinx/aie-rt.git) registered for path 'third_party/aie-rt'
Submodule 'third_party/bootgen' (https://github.com/Xilinx/bootgen.git) registered for path 'third_party/bootgen'
Submodule 'third_party/iree' (https://github.com/iree-org/iree.git) registered for path 'third_party/iree'
Submodule 'third_party/mlir-air' (https://github.com/nod-ai/mlir-air.git) registered for path 'third_party/mlir-air'
Submodule 'third_party/openssl' (https://github.com/viaduck/openssl-cmake.git) registered for path 'third_party/openssl'
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/XRT'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/aie-rt'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/bootgen'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/iree'...
Cloning into '/_work/iree-amd-aie/iree-amd-aie/third_party/mlir-air'...
warning: --depth is ignored in local clones; use file:// instead.
done.

(https://github.com/nod-ai/iree-amd-aie/actions/runs/10671927949/job/29578539280?pr=733)

Ya I saw that and that's why I added -shallow-submodules. I'm not 100% on whether this works correctly but 🤷‍♂️

Yeah, not sure, it says to add file://..., maybe something to try at some point, but it looks like the clone goes reasonably fast.. at least faster than sync_deps.py!

makslevental commented 2 months ago

Yeah, not sure, it says to add file://..., maybe something to try at some point, but it looks like the clone goes reasonably fast.. at least faster than sync_deps.py!

Ya the thing is I have no clue where to add this... Anyway I'll put a TODO there and make an issue