Skip to content

Conversation

@jzc
Copy link
Contributor

@jzc jzc commented Dec 5, 2024

For virtual functions the "construction" kernels that create objects that have function virtual functions but do not actually perform any virtual calls have the calls-indirectly attribute attached to them by the SYCLVirtualFunctionAnalysisPass. Because of this, aspect propagation will attach the aspects used by the sets referenced by the calls-indirectly to the construction kernel. However, these images should be able to be launched regardless of the optional kernel features used by their virtual functions, so this PR updates the aspect propagation logic to only propagate aspects to functions that have virtual calls.

// A "construction" kernel - it is not marked with `calls_indirectly`, does not perform any virtual calls,
// but creates an object with virtual functions
q.single_task([=] {
  // The kernel is not submitted with 'assume_indirect_calls_to' property and therefore
  // it is not considered to be using any of virtual member functions of 'Foo'.
  // This means that the object of 'Foo' can be successfully created by this
  // kernel, regardless of whether a target device supports 'fp64' aspect which
  // is used by 'Foo::foo'.
  // No exceptions are expected to be thrown.
  new (Storage) Foo;
});

@jzc jzc requested a review from a team as a code owner December 5, 2024 15:34
@jzc jzc temporarily deployed to WindowsCILock December 5, 2024 15:35 — with GitHub Actions Inactive
@jzc jzc temporarily deployed to WindowsCILock December 5, 2024 17:09 — with GitHub Actions Inactive
@jzc jzc requested a review from AlexeySachkov December 9, 2024 16:49
// perform any virtual calls have the calls-indirectly attribute
// attached by SYCLVirtualFunctionAnalysis pass. We do not want to
// attach sycl_used_aspects metadata to such kernels.
bool hasVirtualCall = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool hasVirtualCall = false;
bool HasVirtualCall = false;

LLVM Coding Standards

// attached by SYCLVirtualFunctionAnalysis pass. We do not want to
// attach sycl_used_aspects metadata to such kernels.
bool hasVirtualCall = false;
for (const Instruction &I : instructions(F)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is purely a FYI, LLVM has llvm::none_of which works with their ranges if you want to rewrite this into if (none_of(instructions(F), []() { multi-line lambda here }) return;. See STLExtras

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2025

@intel/llvm-gatekeepers please consider merging

@sarnex
Copy link
Contributor

sarnex commented Sep 2, 2025

Really old PR, I don't think this is intended to be merged, closing. Reopen if it should be merged.

@sarnex sarnex closed this Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants