Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make changes for cmake based build of hexagon_remote #237

Merged
merged 6 commits into from
Aug 28, 2023
Merged

Make changes for cmake based build of hexagon_remote #237

merged 6 commits into from
Aug 28, 2023

Conversation

pranavb-ca
Copy link
Contributor

  • Change HL_HEXAGON_SIM_REMOTE and LD_LIBRARY_PATH to reflect new locations of hexagon_sim_remote and libhalide_hexagon_host.so respectively.
  • Add HEXAGON_SDK_ROOT to point to the toplevel of the Hexagon SDK
    @steven-johnson PTAL

@pranavb-ca
Make changes for cmake based build of hexagon_remote
05b8a4b 
- Change HL_HEXAGON_SIM_REMOTE and LD_LIBRARY_PATH to reflect new locations
of hexagon_sim_remote and libhalide_hexagon_host.so respectively.
- Add HEXAGON_SDK_ROOT to point to the toplevel of the Hexagon SDK
@pranavb-ca pranavb-ca mentioned this pull request Jul 11, 2023
Closed
7 tasks
@steven-johnson
Copy link
Contributor

Should I go ahead and land this and/or activate it on the buildbot master? (Not sure if we need to also land a Halide PR first to avoid breakage)

@pranavb-ca
Copy link
Contributor Author

Should I go ahead and land this and/or activate it on the buildbot master? (Not sure if we need to also land a Halide PR first to avoid breakage)

Yes, there is dependency here. We'll need to land Alex's PR halide/Halide#7671 first.
However, I have a question first. This PR of mine relies on the build property HL_HEXAGON_TOOLS but I see some builders don't have that property set. This one, for instance, doesn't have the property set -> https://buildbot.halide-lang.org/master/#/builders/89/builds/2 while this one does https://buildbot.halide-lang.org/master/#/builders/65/builds/2

@steven-johnson
Copy link
Contributor

OK, I will hold off until I hear more from you.

@steven-johnson
Copy link
Contributor

some builders don't have that property set

Correct: at present we don't do any Hexagon building/testing on anything but x86-64-linux machines; the first one you list is arm-32-linux.

@pranavb-ca
Copy link
Contributor Author

pranavb-ca commented Jul 11, 2023 •
edited
Loading

some builders don't have that property set

Correct: at present we don't do any Hexagon building/testing on anything but x86-64-linux machines; the first one you list is arm-32-linux.

Hmm It looks like we do enable the Hexagon backend on platforms other than x86-64.
See https://buildbot.halide-lang.org/master/#/builders/65/builds/15/steps/8/logs/stdio

@steven-johnson
Copy link
Contributor

OK, this looks good to me -- if you think it's ready to land, I will merge it and push it out to the buildbot master, please let me know

@pranavb-ca
Copy link
Contributor Author

OK, this looks good to me -- if you think it's ready to land, I will merge it and push it out to the buildbot master, please let me know

Thank you. I am testing Alex's PR halide/Halide#7671 one last time. Once that is good, I'll confirm here and then this can go in.

@steven-johnson
Copy link
Contributor

Monday Morning Ping -- LMK when you are happy with halide/Halide#7671 (and thus, ready for this to land)

@pranavb-ca
Copy link
Contributor Author

pranavb-ca commented Jul 24, 2023 •
edited
Loading

Monday Morning Ping -- LMK when you are happy with halide/Halide#7671 (and thus, ready for this to land)

Thanks @steven-johnson - Sure, will do this. Ran into some errors testing halide/Halide#7671 late Friday. Coulnd't get to them over the weekend. I'll post an update here.

@pranavb-ca
Copy link
Contributor Author

Merged master to fix conflicts.
@steven-johnson -> halide/Halide#7671 is ok to merge. Once that is merged, we can merge this as well.

@steven-johnson
Copy link
Contributor

We can't merge halide/Halide#7671 until the clang-tidy errors are fixed -- we'll be good to go once that happens

@pranavb-ca pranavb-ca mentioned this pull request Aug 4, 2023
Merged
@pranavb-ca
Copy link
Contributor Author

Since halide/Halide#7741 has been merged, we can merge this PR as well.

@steven-johnson steven-johnson merged commit 50e2cb9 into halide:master Aug 28, 2023
3 checks passed
@steven-johnson
Copy link
Contributor

landed and applied to bots, should be in use now

@pranavb-ca
Copy link
Contributor Author

Thank you.

@pranavb-ca pranavb-ca mentioned this pull request Sep 6, 2023
Open
pranavb-ca added a commit to pranavb-ca/build_bot that referenced this pull request Sep 6, 2023
@pranavb-ca
Revert "Make changes for cmake based build of hexagon_remote (halide#237
cc3ba35 
)"

This reverts commit 50e2cb9.
abadams added a commit that referenced this pull request Sep 6, 2023
@abadams
Merge pull request #264 from pranavb-ca/pdb/revert_237
1cb252c 
Revert "Make changes for cmake based build of hexagon_remote (#237)"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexreinking alexreinking alexreinking approved these changes

@steven-johnson steven-johnson steven-johnson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pranavb-ca @steven-johnson @alexreinking