INT Header value kernel_signature_start has the wrong values

[erichkeane]

See this test for a good example: https://github.com/intel/llvm/blob/e35466be32bd41607a10a39bf36546f7667829fa/clang/test/CodeGenSYCL/kernel-param-pod-array-ih.cpp

Above the kernel_signature_start global, we print the comment:
// indices into the kernel_signatures array, each representing a start of
// kernel signature descriptor subarray of the kernel_signatures array;
// the index order in this array corresponds to the kernel name order in the
// kernel_names array

HOWEVER, the values are wrong, every kernel results in it getting 1-too-many values. In this example, if you count the indices, they should be 0, 5, 11 instead of 0, 6, 13.

This has been broken for AT LEAST 1 month (based on when this test was done), and I have no idea how much longer before then. The KernelInfo structs don't actually use this array for anything, so I suspect that it is unused, we might be able to just remove the printing of this (and any infrastructure).

@romanovvlad : Can you comment as to whether the runtime is using this? Are you OK if we remove that array?

[romanovvlad]

@erichkeane
The SYCL RT doesn't use kernel_signature_start, it only uses methods of KernelDesc class, specifically the following methods:

 static constexpr const char* getName();
 static constexpr unsigned getNumParams()
static constexpr const kernel_param_desc_t& getParamDesc(unsigned i) 

[erichkeane]

@erichkeane
The SYCL RT doesn't use kernel_signature_start, it only uses methods of KernelDesc class, specifically the following methods:

 static constexpr const char* getName();
 static constexpr unsigned getNumParams()
static constexpr const kernel_param_desc_t& getParamDesc(unsigned i) 

Thanks for the confirmation @romanovvlad . I suspected as much, since the kernel_signature_start values have been broken for as long as I can find :)

So @jtmott-intel this is confirmation that you can just remove the field and all code there to support it.

[romanovvlad]

@kbobrovs FYI

[kbobrovs]

I think the history is the following:
Some time ago the kernel_signatures array used kind_none parameter kind as delimiter between signatures of different kernels:

const kernel_param_desc_t kernel_signatures[] = {
  //--- hw_cpp_45_1_SimpleVadd_L__int_4_char_float_p_MyTmplClass_L__MyInnerTmplClass_L__p_p_MyClass__J___J___J_
  { kind_accessor, 2014, 0 },
  { kind_accessor, 2014, 64 },
  { kind_accessor, 2014, 128 },
  { kind_scalar, 4, 192 },
  { kind_none, 0, 0 },
  //--- hw_cpp_45_1_SimpleVadd_L__float_4_int__J_
  { kind_accessor, 2014, 0 },
  { kind_accessor, 2014, 64 },
  { kind_accessor, 2014, 128 },
  { kind_scalar, 4, 192 },
  { kind_none, 0, 0 }
};

// indices into the kernel_signatures array, each representing a start of
// kernel signature descriptor subarray of the kernel_signatures array;
// the index order in this array corresponds to the kernel name order in the
// kernel_names array
const unsigned kernel_signature_start[] = {
  0, // hw_cpp_45_1_SimpleVadd_L__int_4_char_float_p_MyTmplClass_L__MyInnerTmplClass_L__p_p_MyClass__J___J___J_
  5 // hw_cpp_45_1_SimpleVadd_L__float_4_int__J_
};

Later on this kind was removed along with its emission. At the time of the removal RT did not use kernel_signature_start already, but used const kernel_param_desc_t& KernelInfo<KernelIDTy>::getParamDesc(unsigned i), which indexed into kernel_signatures directly.

I'm not sure if kernel_signature_start has ever been used, by IIRC, it was an attempt to enable C clients to include this header and peek into lambda object. We had #ifdef __cplusplus in the int header, at that time too.