Skip to content
Snippets Groups Projects

add d/p/0020-replace-x86_64-with-variables.patch

Merged Cordell Bloor requested to merge cgmb/rocm-hipamd:multiarch into master

This should fix some of the compilation errors on other platforms. I'm not sure if the packages actually work or not, but they compile.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Thanks for the patch, it may not be perfect in cross-compiling context yet (naive me would have been hardwired to put CMAKE_HOST_SYSTEM_PROCESSOR instead), but it will be helpful in improving architecture coverage in the future. I don't suppose the triplet returned by clang -dumpmachine returning x86_64-pc-linux-gnu instead of x86_64-unknown-linux-gnu will be a problem?

    I note some architectures like armel do not have clang_rt.builtins, but to be honest I don't expect many people to want to target that platform anymore.

  • I don't suppose the triplet returned by clang -dumpmachine returning x86_64-pc-linux-gnu instead of x86_64-unknown-linux-gnu will be a problem?

    Good catch. I suppose that perhaps it should just be $(uname -m)-unknown-linux-gnu. Surprisingly, the changes to hip/bin/hip_embed_pch.sh were not actually even needed to get HIP compiling on aarch64.

    Edited by Cordell Bloor
  • To be honest, I asked initially because I did not know. Thinking another time about it, clang -dumpmachine could also have been in position to fill something more precise than unknown and thus could be trusted to do the right thing(TM). Interestingly, cpp --help documents this is the target platform, but does not return the second item of the quartet, just x64_64-linux-gnu; maybe there is a risk of a different definition in Gcc. I'm not sure how to interpret the fact that the build goes through on aarch64 without the patch involving the quartet, but it may highlight that this is needed in quite a corner case.

    In any case, many thanks for considering support on non-x86_64 platforms! :)

    I'll let a good night of sleep other this and merge as-is tomorrow if there are no objections.

  • Cordell Bloor added 1 commit

    added 1 commit

    • d62773aa - Match upstream target triple exactly on x86_64

    Compare with previous version

  • I would prefer $(uname -m)-unknown-linux-gnu as that ensures the command is identical to upstream when on x86_64. I don't fully understand this code, so matching the existing behaviour exactly seems like the lowest-risk option.

  • I understand. The end result sounds good to me, so merging. Thank you for your contribution! :)

  • mentioned in commit cf265fd0

Please register or sign in to reply
Loading