Check if device support heterogeneous memory

[sherry-yuan]

Currently sycl runtime is trying to distinguish whether buffer location is supported on the target device using lower level runtime getDeviceInfo call to determine it. However current runtime get device info does not yet support accepting the buffer location property yet, this change addes it.

The referenced sycl runtime PR: intel/llvm#5604

This change is only needed if sycl runtime cannot come up with an workaround.

[pcolberg]

Thanks @sherry-yuan 🙂

Maybe title the commit Add buffer location to supported extensions since it is always supported?

[sherry-yuan]

I think something is wrong with the windows machine, ninja got uninstalled?

https://github.com/intel/fpga-runtime-for-opencl/runs/5426146169?check_suite_focus=true

     | The term 'ninja' is not recognized as a name of a cmdlet, function, script file, or executable
     | program. Check the spelling of the name, or if a path was included, verify that the path is correct
     | and try again.

[pcolberg]

I think something is wrong with the windows machine, ninja got uninstalled?

https://github.com/intel/fpga-runtime-for-opencl/runs/5426146169?check_suite_focus=true

     | The term 'ninja' is not recognized as a name of a cmdlet, function, script file, or executable
     | program. Check the spelling of the name, or if a path was included, verify that the path is correct
     | and try again.

No worries, this looks like a temporary failure, maybe related to actions/cache. The previous run failed on Windows 2022 with a similar error of aoc missing from PATH.

[pcolberg]

Thanks Sherry! I have revised the commit series slightly while keeping the contents the same.

When committing a change, think of it like a story where each commit contains a different aspect of your change. Rather than a chronological series, try to organise your commits into a logical series, where each commit is as self-contained as possible.

Taking this change as an example, you have three self-contained changes.

1. Checking for size_ret > 0 in all cases. This is independent of the other changes, so you can make this change anywhere in the series; usually I prefer to get these out of the way first so the reviewer can focus on the actual change thereafter.
2. Factoring out and testing the extensions string. Since you want to proof that your desired change, adding the buffer_location extension, does not break the test, this commit should come before the next commit.
3. Now you can add the new extension. Not only have you proven that your change does not break the test, you have also shown that the test does not require any modification to test your new extension string, which was the point of factoring out the extension string.

A fine-grained commit series makes sense when the maintainers value their code history, i.e., merge or rebase pull-request branches as is. Fine-grained commits can be immensely valuable for later reading and comprehension, or easy cherry-picking to release branches, to give only two of many examples. On the other hand, when pull-request branches are merged by squashing down to a single commit, the effort is unfortunately in vain.

[sherry-yuan]

Thanks @pcolberg for the guide!

In this case we would prefer a PR to contain multiple commits where each is a self-contained change rather than squashing all to one commit?

[pcolberg]

In this case we would prefer a PR to contain multiple commits where each is a self-contained change rather than squashing all to one commit?

Yes, because in this PR each commit introduces a new aspect of your change, multiple commits are justified.

On the other hand, consider another example of one commit making a small change and the next commit applying clang-format. In this case, the two commits should be squashed, since the clang-format is nothing but noise to a reviewer, it does not add anything new to their understanding of the change.

If in doubt, I would always tend to more commits rather than fewer; commits can always be squashed if desired, but unsquashing a commit requires more work.

[sherry-yuan]

I see, thanks!