Implement runtime support for device global with init_mode reprogram

[sophimao]

• Change to autodiscovery string: add fields in device global for device global attributes, including host_access, init_mode, and implement_in_csr
• Change to kernel: if the device to which kernel is to be launched contains device global with init_mode reprogram, force reprogram device before kernel launch

[sophimao]

Compiler side change is in the move now, marking this as ready for review.

[pcolberg]

Thanks @sophimao! The overall change looks good, just some minor stylistic comments.

Could you restructure this into two commits, folding in the clang-format change?

1. Extend auto-discovery string parsing and unit-test.
2. Extend kernel reprogramming logic.

For the second commit, is it feasible to add unit testing? If it is not obvious, what do you see as the main blocker?

[pcolberg]

@sophimao, does this PR fully resolve #56?

[sophimao]

@sophimao, does this PR fully resolve #56?

Unfortunately no, #56 tracks the device global copy kernel, which is in PR #65. That PR is also waiting on compiler support.

[sophimao]

For the second commit, is it feasible to add unit testing? If it is not obvious, what do you see as the main blocker?

I looked into adding unit testing for the kernel change, to do that I would need to add another device with device global, that would enable me to run through the modified code, but I don't see a way to ensure the need_reprogram variable to be true (I don't think we can unit test reprogram either). Would that be okay?

Edit: the unit test seems to have a construct for testing device op queue status acl_device_op_test_ctx_t, will see if I can use that to write a unit test

[pcolberg]

the unit test seems to have a construct for testing device op queue status acl_device_op_test_ctx_t, will see if I can use that to write a unit test

Yup, the effect of need_program is to submit a ACL_DEVICE_OP_REPROGRAM, which you should be able to verify in the unit test, depending on the availability of device globals and the binary_rand_hash. The acl_kernel_reprogram_scheduler test group should provide ideas on how to implement this.

[pcolberg]

Thanks @sophimao for the revision. Apart from a few minor details, this is ready to be merged once the corresponding compiler change has been tested together with this change.

[pcolberg]

Thanks @sophimao, this is good to be merged once integration testing is completed.

@ChenXingYang2000, you can use the runtime commit hash daa9492 for your testing.

[sophimao]

Hi @ChenXingYang2000, I have updated the code and sanity checked with the reprogram_trivial test you provided. I might add some more unit tests later but you can use hash 2ad0b64 for testing now.

[ChenXingYang2000]

Hi @ChenXingYang2000, I have updated the code and sanity checked with the reprogram_trivial test you provided. I might add some more unit tests later but you can use hash 2ad0b64 for testing now.

Thank you for such quick fix, really appreciate it, I will test it out now.

[sophimao]

Hi @pcolberg , I'm done changing the code for now to align with the desired behaviour of this feature, would you mind take a look at this again when you have time? For testing purposes I pushed the last two commits directly but I'll squash them when this change is fully ready. Thank you!

[pcolberg]

Thanks @sophimao! Most of this change is dedicated to new tests which is great 🙂

[pcolberg]

Thanks @sophimao!