[llvm] [SYCL][LLVM] Adding property set I/O library for SYCL (PR #110771)

Arvind Sudarsanam via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 21 10:42:22 PDT 2024


asudarsa wrote:

> > One of my colleagues had tried to switch to YAML some time ago, but that effort got stuck pretty quickly. The main problem that we encountered with switching to YAML was lack of flexibility: we don't know how many SYCL properties there will be and SYCL properties within a single set (which could have been represented as an array) may have different types (which makes it hard to represent them as an array).
> 
> YAML files can contain multiple "documents" as well. I'm not sure I see how YAML is unsuitable for this use case.
> 
> That aside, it really seems to me that this design decision has a bunch of different implications.
> 
> If we go forward with this solution I think we need _a lot_ more tests of the parser itself since parsing is generally a source of security bugs.

Hi @llvm-beanz 

Thanks for your feedback. Just a heads up. I am currently looking at adding more tests.
A quick addon comment. One of our primary objective is to provide a fully functional SYCL offloading support and PropertySetIO plays a small, but important role in our compilation flow. So, it will be great to have this upstreamed to unblock other parts of our upstreaming process. One good source of confidence for our implementation is that it has been present in our intel/llvm compiler for a while. However, I completely agree with your pointer that this will benefit from additional testing.

Thanks



https://github.com/llvm/llvm-project/pull/110771


More information about the llvm-commits mailing list