[SYCL] Support USM buffer location property in malloc_shared

[pcolberg]

This ports commit 12c988a from malloc_device to malloc_shared for use with the FPGA Runtime for OpenCL.

See malloc_device implementation in #5634

See SYCL extension specification in #6219

@tiwaria1 @bsyrowik @GarveyJoe @aditikum @ajaykumarkannan

[tiwaria1]

buffer_location property spec update is here: #6219

[bsyrowik]

Amazing, thank you Peter!

[pcolberg]

@intel/llvm-reviewers-runtime Are the failures in L0 GEN9 and HIP AMDGPU related to this PR? I am unsure how to interpret the test output.

[cperkinsintel]

@pcolberg I restarted the test run. @steffenlarsen only had a moment to look at it very briefly and mentioned a theory about Hip and L0 not handling empty property lists properly, but that was a bit rushed, off the cuff. Does that remark contain any clue for yourself?

[pcolberg]

@pcolberg I restarted the test run. @steffenlarsen only had a moment to look at it very briefly and mentioned a theory about Hip and L0 not handling empty property lists properly, but that was a bit rushed, off the cuff. Does that remark contain any clue for yourself?

Thanks @cperkinsintel and @steffenlarsen! Yes, that explains the failures, I am indeed always passing an empty property list. I will rework the change to pass nullptr instead.

[steffenlarsen]

Apologies for the inconvenience, @pcolberg. The problem with empty lists of properties should be fixed with #6225.

[tiwaria1]

Hi @steffenlarsen is this PR now good for merging?

[pcolberg]

@steffenlarsen Thanks for adressing this so quickly 🙂

Which variant of the change would you prefer, the single case with empty or nonempty null-terminated list, or the two cases with either null pointer or null-terminated nonempty list? I tend to branchless code for readability. On the other hand, the heap allocation in the absence of properties is not that pretty either; not that the overhead matters for this high-level call, just in principle.

[pcolberg]

@tiwaria1 Apart from the pending review, we should align this PR with #6220, which still fails in L0 for unknown reason #6220 (comment). If the resolution requires a design change of the implementation, this PR should be modified in the same way.

[tiwaria1]

ping @cperkinsintel @intel/llvm-reviewers-runtime , please review the change.

[pcolberg]

ESIMD Emu runner is broken, unrelated to this PR:

##[error]Path does not exist /localdisk2/github/_work/llvm/llvm/build/results_esimd_emu_default.json
##[error]Exit code 1 returned from process: file name '/localdisk2/github/bin.2.292.0/Runner.PluginHost', arguments 'action "GitHub.Runner.Plugins.Artifact.PublishArtifact, Runner.Plugins"'.

Update: This appears to be an infrastructure issue and has since been resolved.

[steffenlarsen]

Which variant of the change would you prefer, the single case with empty or nonempty null-terminated list, or the two cases with either null pointer or null-terminated nonempty list? I tend to branchless code for readability. On the other hand, the heap allocation in the absence of properties is not that pretty either; not that the overhead matters for this high-level call, just in principle.

I agree, the branchless version was cleaner. If you want to minimize the heap allocation, you could make a static sized array with the maximum number of possible properties, then insert 0 after the last present property.

[pcolberg]

If you want to minimize the heap allocation, you could make a static sized array with the maximum number of possible properties, then insert 0 after the last present property.

Thanks @steffenlarsen, I have changed the properties list to a stack array.

@cperkinsintel This PR is now ready for review and merge.

[steffenlarsen]

LGTM!

[pcolberg]

@intel/llvm-gatekeepers This PR is ready for merge.