From 5ae62069826e62d123765c65eeb17e86e9d6165e Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Wed, 5 Jun 2024 04:23:15 -0700 Subject: [PATCH 1/3] [SYCL] Annotate virtual calls in device IR This commit is a part of virtual functions extension implementation that is proposed in intel/llvm#10540. As part of the feature, we would like to achieve both: - allow virtual function calls from SYCL kernels that are submitted with the right properties as described in the language extension specification - disallow virtual function calls from SYCL kernels that do not use any (or the right) properties to conform with the core SYCL 2020 specification Implementing such diagnostics will require call graph traversal to understand the origin of the call and that is not what's easy to do in FE. Therefore, the implementation design for virtual functions proposes that such diagnostic is offloaded to a pass. This commit is a preparations for it: it introduces an attribute to all virtual function calls in device code so they can be easily detected and not confused with plain function pointer calls (for which we don't have a language specification and therefore well-defined behavior yet). --- clang/lib/CodeGen/CGCall.cpp | 8 ++++ .../CodeGenSYCL/attrs-on-virtual-calls.cpp | 42 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index b2d0d5edc9232..ff360dad0ff69 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -5713,6 +5713,14 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, // Apply some call-site-specific attributes. // TODO: work this into building the attribute set. + if (getContext().getLangOpts().SYCLIsDevice && Callee.isVirtual()) { + // Annotate virtual calls in SYCL device code to help passes that emit + // diagnostics on incorrect uses of virtual functions. + Attrs = Attrs.addFnAttribute( + getLLVMContext(), + llvm::Attribute::get(getLLVMContext(), "virtual-call")); + } + // Apply always_inline to all calls within flatten functions. // FIXME: should this really take priority over __try, below? if (CurCodeDecl && CurCodeDecl->hasAttr() && diff --git a/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp b/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp new file mode 100644 index 0000000000000..c5ee881e409d1 --- /dev/null +++ b/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp @@ -0,0 +1,42 @@ +// Test verifies that clang codegen properly adds call site attributes to +// device code + +// RUN: %clang_cc1 -triple spir64 -fsycl-allow-virtual-functions \ +// RUN: -fsycl-is-device -emit-llvm %s -o %t.device +// RUN: FileCheck %s --input-file=%t.device --check-prefixes=CHECK,CHECK-DEVICE +// RUN: %clang_cc1 -triple x86_64 -fsycl-allow-virtual-functions \ +// RUN: -fsycl-is-host -emit-llvm %s -o %t.host +// RUN: FileCheck %s --input-file=%t.host --check-prefixes=CHECK,CHECK-HOST + +// CHECK-LABEL: define {{.*}} @_Z4testv +// CHECK: call{{.*}}void %[[#]](ptr +// CHECK-DEVICE-SAME: #[[#ATTRS:]] +// CHECK-HOST-SAME: %[[#]]){{[[:space:]]}} +// CHECK-DEVICE: attributes #[[#ATTRS]] = {{.*}} "virtual-call" + +#ifndef SYCL_EXTERNAL +#define SYCL_EXTERNAL +#endif + +SYCL_EXTERNAL bool rand(); + +class Base { +public: + [[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "")]] + virtual void display() {} +}; + +class Derived1 : public Base { +public: + [[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "")]] + void display() override {} +}; + +SYCL_EXTERNAL void test() { + Derived1 d1; + Base *b = nullptr; + if (rand()) + b = &d1; + b->display(); +} + From 58789f83ed92eb396505dca2bbc0ea260f6bbb2d Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Tue, 11 Jun 2024 07:27:23 -0700 Subject: [PATCH 2/3] Enchance the test case --- clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp b/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp index c5ee881e409d1..22ad9d13d3119 100644 --- a/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp +++ b/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp @@ -12,6 +12,8 @@ // CHECK: call{{.*}}void %[[#]](ptr // CHECK-DEVICE-SAME: #[[#ATTRS:]] // CHECK-HOST-SAME: %[[#]]){{[[:space:]]}} +// CHECK: call{{.*}}void @_ZN8Derived17displayEv{{.*}}[[#DIRECT_CALL_ATTRS:]] +// CHECK-NOT: attributes #[[#DIRECT_CALL_ATTRS]]{{.*}}"virtual-call" // CHECK-DEVICE: attributes #[[#ATTRS]] = {{.*}} "virtual-call" #ifndef SYCL_EXTERNAL @@ -38,5 +40,6 @@ SYCL_EXTERNAL void test() { if (rand()) b = &d1; b->display(); + d1.display(); } From b532d00d19ee733e4408a848642586e0973a9086 Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Fri, 5 Jul 2024 08:04:23 -0700 Subject: [PATCH 3/3] Expand testing --- .../CodeGenSYCL/attrs-on-virtual-calls.cpp | 104 ++++++++++++++---- 1 file changed, 85 insertions(+), 19 deletions(-) diff --git a/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp b/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp index 22ad9d13d3119..a8503f5b0df6b 100644 --- a/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp +++ b/clang/test/CodeGenSYCL/attrs-on-virtual-calls.cpp @@ -3,18 +3,12 @@ // RUN: %clang_cc1 -triple spir64 -fsycl-allow-virtual-functions \ // RUN: -fsycl-is-device -emit-llvm %s -o %t.device -// RUN: FileCheck %s --input-file=%t.device --check-prefixes=CHECK,CHECK-DEVICE +// RUN: FileCheck %s --input-file=%t.device // RUN: %clang_cc1 -triple x86_64 -fsycl-allow-virtual-functions \ // RUN: -fsycl-is-host -emit-llvm %s -o %t.host -// RUN: FileCheck %s --input-file=%t.host --check-prefixes=CHECK,CHECK-HOST +// RUN: FileCheck %s --input-file=%t.host --check-prefix=CHECK-HOST -// CHECK-LABEL: define {{.*}} @_Z4testv -// CHECK: call{{.*}}void %[[#]](ptr -// CHECK-DEVICE-SAME: #[[#ATTRS:]] -// CHECK-HOST-SAME: %[[#]]){{[[:space:]]}} -// CHECK: call{{.*}}void @_ZN8Derived17displayEv{{.*}}[[#DIRECT_CALL_ATTRS:]] -// CHECK-NOT: attributes #[[#DIRECT_CALL_ATTRS]]{{.*}}"virtual-call" -// CHECK-DEVICE: attributes #[[#ATTRS]] = {{.*}} "virtual-call" +// CHECK-HOST-NOT: attributes {{.*}} "virtual-call" #ifndef SYCL_EXTERNAL #define SYCL_EXTERNAL @@ -24,22 +18,94 @@ SYCL_EXTERNAL bool rand(); class Base { public: - [[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "")]] virtual void display() {} }; -class Derived1 : public Base { +class Derived : public Base { public: - [[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "")]] void display() override {} + + // CHECK-LABEL: define {{.*}} @_ZN7Derived3foo + // CHECK: call {{.*}}void %[[#]]{{.*}} #[[#VIRTUAL_CALL_ATTR:]] + // CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR:]] + void foo() { + display(); // virtual call + Base::display(); // non-virtual call + } }; -SYCL_EXTERNAL void test() { - Derived1 d1; - Base *b = nullptr; - if (rand()) - b = &d1; - b->display(); - d1.display(); +SYCL_EXTERNAL void test_calls_with_implicit_this() { + Derived d; + d.foo(); +} + +// CHECK-LABEL: define {{.*}} @_Z21test_base_ref_to_base +// CHECK: call {{.*}}void %[[#]]{{.*}} #[[#VIRTUAL_CALL_ATTR]] +// CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR]] +SYCL_EXTERNAL void test_base_ref_to_base() { + Base b; + Base &br = b; + + br.display(); // virtual call + br.Base::display(); // non-virtual call +} + +// CHECK-LABEL: define {{.*}} @_Z24test_base_ref_to_derived +// CHECK: call {{.*}}void %[[#]]{{.*}} #[[#VIRTUAL_CALL_ATTR]] +// CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR]] +SYCL_EXTERNAL void test_base_ref_to_derived() { + Derived d; + Base &br = d; + + br.display(); // virtual call + br.Base::display(); // non-virtual call +} + +// CHECK-LABEL: define {{.*}} @_Z21test_base_ptr_to_base +// CHECK: call {{.*}}void %[[#]]{{.*}} #[[#VIRTUAL_CALL_ATTR]] +// CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR]] +SYCL_EXTERNAL void test_base_ptr_to_base() { + Base b; + Base *bp = &b; + + bp->display(); // virtual call + bp->Base::display(); // non-virtual call +} + +// CHECK-LABEL: define {{.*}} @_Z24test_base_ptr_to_derived +// CHECK: call {{.*}}void %[[#]]{{.*}} #[[#VIRTUAL_CALL_ATTR]] +// CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR]] +SYCL_EXTERNAL void test_base_ptr_to_derived() { + Derived d; + Base *bp = &d; + + bp->display(); // virtual call + bp->Base::display(); // non-virtual call +} + +// CHECK-LABEL: define {{.*}} @_Z9test_base +// CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR]] +// CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR]] +SYCL_EXTERNAL void test_base() { + Base b; + + // Strictly speaking, this is a virtual function call, but clang emits it + // as a direct call + b.display(); + b.Base::display(); // non-virtual call +} + +// CHECK-LABEL: define {{.*}} @_Z12test_derived +// CHECK: call {{.*}}void @_ZN7Derived7display{{.*}} #[[#DIRECT_CALL_ATTR]] +// CHECK: call {{.*}}void @_ZN4Base7display{{.*}} #[[#DIRECT_CALL_ATTR]] +SYCL_EXTERNAL void test_derived() { + Derived d; + + // Strictly speaking, this is a virtual function call, but clang emits it + // as a direct call + d.display(); + d.Base::display(); // non-virtual call } +// CHECK-NOT: attributes #[[#DIRECT_CALL_ATTR]] = {{.*}}"virtual-call" +// CHECK: attributes #[[#VIRTUAL_CALL_ATTR]] = {{.*}}"virtual-call"