llvm
llvm copied to clipboard
intel
[SYCL] Make PropertySetRegistry being owning it's content
Before this patch PropertySetRegistry used StringRefs that point at external data that was supposed to outlive the instance of PropertySetRegistry. It wasn't obvious and it might lead to memory bugs. This patch makes PropertySetRegistry owning it's content as containers usually do.
Also documentation is aligned with doxygen BKSM.
From test's failures it looks like we can't simply change the order of spec const's IDs in metadata.
Status update: Initially I made a change that removes the order of elements in PropertyRegistry. Later on I found out that the current SpecConsts pass rely on that property and this is difficult to adopt the pass to the missing order of elements or sorted order of elements. I decided to return this property back. This PR still has a fix of data owning that removes UB with use of freed data.
Hi @maksimsab
I think such changes to core LLVM files should be directly made upstream, I would advise to split this into two PR. One upstream PR to change code inside PropertySetIO files and once that change is done, you can make the changes to sycl-post-link on top of it.
Thanks
Hi @asudarsa
PropertySetIO doesn't belong to llvm-project.
Oh. You are right. Sorry a miss on my part. One more thing to upstream then
The pre-commit failure is known and has been reported already in https://github.com/intel/llvm/pull/12791. I don't think it's related to these changes.
