[SYCL] Fix crash on array of pointers

[elizabethandrews]

The crash happens when address space of pointer element is
checked using array type. The type of 'array element' should
be checked, not the 'array type' itself. Also added a missing
LValueToRValue cast for pointers.

Signed-off-by: Elizabeth Andrews [email protected]

[erichkeane]

Cool, easier than I thought :)

Does this work for arrays-of-pointers in the 'root' lambda? So something like:

void foo() {
  int *array[2];
  kernel<class whatever>([=](){ return array[0];});
}

[elizabethandrews]

Cool, easier than I thought :)

Does this work for arrays-of-pointers in the 'root' lambda? So something like:

void foo() {
  int *array[2];
  kernel<class whatever>([=](){ return array[0];});
}

I think so. I will add a test for that as well. Unfortunately though, I was just running some tests locally and it crashed for the 2D array of pointers in CodeGen :/ I'm not sure why check-all didn't show this when I tested it earlier. The runtime test should have crashed.

[erichkeane]

Cool, easier than I thought :)
Does this work for arrays-of-pointers in the 'root' lambda? So something like:

void foo() {
  int *array[2];
  kernel<class whatever>([=](){ return array[0];});
}

I think so. I will add a test for that as well. Unfortunately though, I was just running some tests locally and it crashed for the 2D array of pointers in CodeGen :/ I'm not sure why check-all didn't show this when I tested it earlier. The runtime test should have crashed.

Well, the question is more, "Are they wrapped" (I don't think so), and should they be?

[elizabethandrews]

Well, the question is more, "Are they wrapped" (I don't think so), and should they be?

No they aren't. And they shouldn't be because that affects current USM functionality. We're maintaining struct depth to avoid this. The wrapping was added in a prior commit to get rid of runtime errors which came as a result of decomposition for unused pointers in structs. These should just be ignored by compiler (since they're not used). If they ever are incorrectly used it'll crash/error in backend.

[vladimirlaz]

Cool, easier than I thought :)
Does this work for arrays-of-pointers in the 'root' lambda? So something like:

void foo() {
  int *array[2];
  kernel<class whatever>([=](){ return array[0];});
}

I think so. I will add a test for that as well. Unfortunately though, I was just running some tests locally and it crashed for the 2D array of pointers in CodeGen :/ I'm not sure why check-all didn't show this when I tested it earlier. The runtime test should have crashed.

@elizabethandrews, check-sycl is not a not part of check-all.

[elizabethandrews]

Cool, easier than I thought :)
Does this work for arrays-of-pointers in the 'root' lambda? So something like:

void foo() {
  int *array[2];
  kernel<class whatever>([=](){ return array[0];});
}

I think so. I will add a test for that as well. Unfortunately though, I was just running some tests locally and it crashed for the 2D array of pointers in CodeGen :/ I'm not sure why check-all didn't show this when I tested it earlier. The runtime test should have crashed.

@elizabethandrews, check-sycl is not a not part of check-all.

Oh really? That explains it then. I've always run check-all to make sure tests are passing but I guess I need to run check-all and check-sycl. Thanks for letting me know!

[elizabethandrews]

@intel/llvm-reviewers-runtime there is a runtime test change. It only adds unused array of pointers to a struct to confirm the code does not crash in this case.

[alexbatashev]

Runtime changes LGTM

[elizabethandrews]

Thanks for the reviews everyone! @bader this is ready for your review/merge.