[DeviceSanitizer] Fix device global type of KernelMetadata #16357
Conversation
AllanZyne
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
AllanZyne
changed the title
|
Hi @intel/dpcpp-tools-reviewers, please review, thanks! |
AllanZyne
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| KernelMetadata->copyAttributesFrom(KernelMetadataOld); | ||
| KernelMetadata->copyMetadata(KernelMetadataOld, 0); | ||
|
|
||
| KernelMetadataOld->eraseFromParent(); |
There was a problem hiding this comment.
why not generate KernelMetadataOld in struct type in the first place?
There was a problem hiding this comment.
This is a good question.
"KernelMetadata" is an array with appending linkage, so it can't be struct type at beginning.
I also considered to generate "KernelMetadata" at sycl-post-link step completely, but I use "KernelMetadata" to distinguish if the device image is a sanitized module at the same time. If we don't add "KernelMetadata" at sanitizer pass, it's possible that a sanitized module (but without any sanitized kernels) won't enable sanitizer layer at UR.
This is a design decision that if user uses "-fsanitize=" to compile program, we will always enable sanitizer layer in UR.
|
Kindly ping @intel/dpcpp-tools-reviewers. Thanks! |
AllanZyne
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AllanZyne
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| auto Name = KernelMetadata->getName().str(); | ||
| KernelMetadata->setName(Name + "_del"); | ||
| auto *KernelMetadataOld = KernelMetadata; |
There was a problem hiding this comment.
There is Value::takeName which will simplify name transfer
| M, StructTypeWithArray, false, GlobalValue::ExternalLinkage, | ||
| ConstantStruct::get(StructTypeWithArray, | ||
| KernelMetadataOld->getInitializer()), | ||
| Name, nullptr, GlobalValue::NotThreadLocal, 1); |
There was a problem hiding this comment.
Would be good to have an inline comment explaining this magic 1
AllanZyne
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AllanZyne
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
@intel/llvm-gatekeepers, please merge. Thanks! |
|
Kindly ping @intel/llvm-gatekeepers , please help merge the PR, thanks. |
OpenCL CPU requires the type of device global wraps with structure type