Skip to content

Conversation

@linehill
Copy link
Contributor

Preparing HIPSPV toolchain to support --offload-new-driver.

TODOs:

  • update HIPSPV tests, check tests for amdgcn-amd-amdhsa aren't regressing.
  • fix resolving remaining chipStar regressions on switching to new offload driver.

break;
case llvm::Triple::spirv32:
case llvm::Triple::spirv64:
if (Target.getOSName() == "hipspv" ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this here? The linker wrapper passes --target=spirv64-amd-amdhsa which keys off of the HSA OS above. Do we need separate handling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what is being asked here. chipStar utilizes HIPSPV TC which is achieved by keying off with the spirv*-*-chipstar triple.

OPT_v,
OPT_cuda_path_EQ,
OPT_rocm_path_EQ,
OPT_hip_path_EQ,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the HIP path really different from the rocm path? Figured those would just be aliases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily? HIP is a subset of ROCm and I think that HIP is a subdirectory in ROCm installations so --hip-path != --rocm-path.

A->getValue(1)));
}
Args.ClaimAllArgs(options::OPT_Xoffload_linker);
Args.ClaimAllArgs(options::OPT_Xoffload_compiler);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where'd the other one go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake - I'll fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants