[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
This revision was automatically updated to reflect the committed changes.
Closed by commit rL349890: [Sema] Produce diagnostics when C++17 aligned
allocation/deallocation (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47757?vs=179195&id=179244#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D47757/new/
https://reviews.llvm.org/D47757
Files:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/CXX/drs/dr2xx.cpp
cfe/trunk/test/SemaCXX/unavailable_aligned_allocation.cpp
Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -8288,6 +8288,7 @@
}
}
+ DiagnoseUseOfDecl(OperatorDelete, Loc);
MarkFunctionReferenced(Loc, OperatorDelete);
Destructor->setOperatorDelete(OperatorDelete, ThisArg);
}
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -66,6 +66,12 @@
if (getLangOpts().CPlusPlus14 && FD->getReturnType()->isUndeducedType() &&
DeduceReturnType(FD, SourceLocation(), /*Diagnose*/ false))
return false;
+
+// See if this is an aligned allocation/deallocation function that is
+// unavailable.
+if (TreatUnavailableAsInvalid &&
+isUnavailableAlignedAllocationFunction(*FD))
+ return false;
}
// See if this function is unavailable.
@@ -228,6 +234,8 @@
// The function 'main' shall not be used within a program.
if (cast(D)->isMain())
Diag(Loc, diag::ext_main_used);
+
+diagnoseUnavailableAlignedAllocation(*cast(D), Loc);
}
// See if this is an auto-typed variable whose initializer we are parsing.
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -1744,28 +1744,33 @@
return false;
}
-// Emit a diagnostic if an aligned allocation/deallocation function that is not
-// implemented in the standard library is selected.
-static void diagnoseUnavailableAlignedAllocation(const FunctionDecl &FD,
- SourceLocation Loc, bool IsDelete,
- Sema &S) {
- if (!S.getLangOpts().AlignedAllocationUnavailable)
-return;
-
- // Return if there is a definition.
+bool
+Sema::isUnavailableAlignedAllocationFunction(const FunctionDecl &FD) const {
+ if (!getLangOpts().AlignedAllocationUnavailable)
+return false;
if (FD.isDefined())
-return;
-
+return false;
bool IsAligned = false;
- if (FD.isReplaceableGlobalAllocationFunction(&IsAligned) && IsAligned) {
-const llvm::Triple &T = S.getASTContext().getTargetInfo().getTriple();
+ if (FD.isReplaceableGlobalAllocationFunction(&IsAligned) && IsAligned)
+return true;
+ return false;
+}
+
+// Emit a diagnostic if an aligned allocation/deallocation function that is not
+// implemented in the standard library is selected.
+void Sema::diagnoseUnavailableAlignedAllocation(const FunctionDecl &FD,
+SourceLocation Loc) {
+ if (isUnavailableAlignedAllocationFunction(FD)) {
+const llvm::Triple &T = getASTContext().getTargetInfo().getTriple();
StringRef OSName = AvailabilityAttr::getPlatformNameSourceSpelling(
-S.getASTContext().getTargetInfo().getPlatformName());
+getASTContext().getTargetInfo().getPlatformName());
-S.Diag(Loc, diag::err_aligned_allocation_unavailable)
+OverloadedOperatorKind Kind = FD.getDeclName().getCXXOverloadedOperator();
+bool IsDelete = Kind == OO_Delete || Kind == OO_Array_Delete;
+Diag(Loc, diag::err_aligned_allocation_unavailable)
<< IsDelete << FD.getType().getAsString() << OSName
<< alignedAllocMinVersion(T.getOS()).getAsString();
-S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable);
+Diag(Loc, diag::note_silence_aligned_allocation_unavailable);
}
}
@@ -2149,13 +2154,11 @@
if (DiagnoseUseOfDecl(OperatorNew, StartLoc))
return ExprError();
MarkFunctionReferenced(StartLoc, OperatorNew);
-diagnoseUnavailableAlignedAllocation(*OperatorNew, StartLoc, false, *this);
}
if (OperatorDelete) {
if (DiagnoseUseOfDecl(OperatorDelete, StartLoc))
return ExprError();
MarkFunctionReferenced(StartLoc, OperatorDelete);
-diagnoseUnavailableAlignedAllocation(*OperatorDelete, StartLoc, true, *this);
}
// C++0x [expr.new]p17:
@@ -3405,8 +3408,7 @@
}
}
-diagnoseUnavailableAlignedAllocation(*OperatorDelete, StartLoc,
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
vsapsai accepted this revision. vsapsai added a comment. The change itself looks correct. Cannot really tell if you need to make changes in other places. For that I rely on Richard's opinion. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47757/new/ https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak updated this revision to Diff 179195.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Check whether the declaration passed to Sema::CanUseDecl is an aligned
allocation/deallocation function that is unavailable.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D47757/new/
https://reviews.llvm.org/D47757
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
test/CXX/drs/dr2xx.cpp
test/SemaCXX/unavailable_aligned_allocation.cpp
Index: test/SemaCXX/unavailable_aligned_allocation.cpp
===
--- test/SemaCXX/unavailable_aligned_allocation.cpp
+++ test/SemaCXX/unavailable_aligned_allocation.cpp
@@ -124,7 +124,73 @@
// expected-note@-20 2 {{if you supply your own aligned allocation functions}}
#endif
-// No errors if user-defined aligned allocation functions are available.
+// Test that diagnostics are produced when an unavailable aligned deallocation
+// function is called from a deleting destructor.
+struct alignas(256) OveralignedS2 {
+ int a[4];
+ virtual ~OveralignedS2();
+};
+
+OveralignedS2::~OveralignedS2() {}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+#if defined(IOS)
+// expected-error@-6 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on iOS 11 or newer}}}
+// expected-note@-7 {{if you supply your own aligned allocation functions}}
+#elif defined(TVOS)
+// expected-error@-9 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on tvOS 11 or newer}}}
+// expected-note@-10 {{if you supply your own aligned allocation functions}}
+#elif defined(WATCHOS)
+// expected-error@-12 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on watchOS 4 or newer}}}
+// expected-note@-13 {{if you supply your own aligned allocation functions}}
+#else
+// expected-error@-15 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on macOS 10.13 or newer}}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+#endif
+#endif
+
+void testExplicitOperatorNewDelete() {
+ void *p = operator new(128);
+ operator delete(p);
+ p = operator new[](128);
+ operator delete[](p);
+ p = __builtin_operator_new(128);
+ __builtin_operator_delete(p);
+}
+
+void testExplicitOperatorNewDeleteOveraligned() {
+ void *p = operator new(128, (std::align_val_t)64);
+ operator delete(p, (std::align_val_t)64);
+ p = operator new[](128, (std::align_val_t)64);
+ operator delete[](p, (std::align_val_t)64);
+ p = __builtin_operator_new(128, (std::align_val_t)64);
+ __builtin_operator_delete(p, (std::align_val_t)64);
+}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+// expected-error@-11 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-12 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-13 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-14 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-15 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-17 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-18 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-19 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-20 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-21 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-22 {{if you supply your own aligned allocation functions}}
+#endif
+
void *operator new(std::size_t __sz, std::align_val_t) {
static char array[256];
return &array;
Index: test/CXX/drs/dr2xx.cpp
===
--- test/CXX/drs/dr2xx.cpp
+++ test/CXX/drs/dr2xx.cpp
@@ -718,7 +718,7 @@
A() {}
};
- // FIXME: These are ill-formed, with a required diagnostic, for the same
+ // FIXME: This is ill-formed, with a required diagnostic, for the same
// reason.
struct B {
inline void operator delete(void*) __attribute__((unused));
@@ -726,7 +726,7 @@
};
struct C {
inline void operator delete(void*) __attribute__((unused));
-virtual ~C() {}
+virtual ~C() {} // expected-warning {{'operator delete' was marked unused but was used}}
};
struct D {
Index: lib
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Sema/SemaExpr.cpp:54
/// emitting diagnostics.
bool Sema::CanUseDecl(NamedDecl *D, bool TreatUnavailableAsInvalid) {
// See if this is an auto-typed variable whose initializer we are parsing.
Does this also need to be updated?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D47757/new/
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47757/new/ https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak updated this revision to Diff 176214.
ahatanak added a comment.
Herald added a subscriber: jkorous.
Rebase & ping.
I've reverted the changes I made to test/SemaCUDA/call-host-fn-from-device.cu
since r342749 fixed the overload resolution bug.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D47757/new/
https://reviews.llvm.org/D47757
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
test/CXX/drs/dr2xx.cpp
test/SemaCXX/unavailable_aligned_allocation.cpp
Index: test/SemaCXX/unavailable_aligned_allocation.cpp
===
--- test/SemaCXX/unavailable_aligned_allocation.cpp
+++ test/SemaCXX/unavailable_aligned_allocation.cpp
@@ -124,7 +124,73 @@
// expected-note@-20 2 {{if you supply your own aligned allocation functions}}
#endif
-// No errors if user-defined aligned allocation functions are available.
+// Test that diagnostics are produced when an unavailable aligned deallocation
+// function is called from a deleting destructor.
+struct alignas(256) OveralignedS2 {
+ int a[4];
+ virtual ~OveralignedS2();
+};
+
+OveralignedS2::~OveralignedS2() {}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+#if defined(IOS)
+// expected-error@-6 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on iOS 11 or newer}}}
+// expected-note@-7 {{if you supply your own aligned allocation functions}}
+#elif defined(TVOS)
+// expected-error@-9 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on tvOS 11 or newer}}}
+// expected-note@-10 {{if you supply your own aligned allocation functions}}
+#elif defined(WATCHOS)
+// expected-error@-12 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on watchOS 4 or newer}}}
+// expected-note@-13 {{if you supply your own aligned allocation functions}}
+#else
+// expected-error@-15 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on macOS 10.13 or newer}}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+#endif
+#endif
+
+void testExplicitOperatorNewDelete() {
+ void *p = operator new(128);
+ operator delete(p);
+ p = operator new[](128);
+ operator delete[](p);
+ p = __builtin_operator_new(128);
+ __builtin_operator_delete(p);
+}
+
+void testExplicitOperatorNewDeleteOveraligned() {
+ void *p = operator new(128, (std::align_val_t)64);
+ operator delete(p, (std::align_val_t)64);
+ p = operator new[](128, (std::align_val_t)64);
+ operator delete[](p, (std::align_val_t)64);
+ p = __builtin_operator_new(128, (std::align_val_t)64);
+ __builtin_operator_delete(p, (std::align_val_t)64);
+}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+// expected-error@-11 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-12 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-13 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-14 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-15 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-17 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-18 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-19 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-20 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-21 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-22 {{if you supply your own aligned allocation functions}}
+#endif
+
void *operator new(std::size_t __sz, std::align_val_t) {
static char array[256];
return &array;
Index: test/CXX/drs/dr2xx.cpp
===
--- test/CXX/drs/dr2xx.cpp
+++ test/CXX/drs/dr2xx.cpp
@@ -718,7 +718,7 @@
A() {}
};
- // FIXME: These are ill-formed, with a required diagnostic, for the same
+ // FIXME: This is ill-formed, with a required diagnostic, for the same
// reason.
struct B {
inline void operator delete(void*) __attribute__((unused));
@@ -726,7 +726,7 @@
};
struct C {
inline void operator delete(void*) __attribute__((unused));
-virtual ~C() {}
+virtual ~C() {} // expected-warning {{'operator delete' was marked unused but was used}}
};
struct D {
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. In https://reviews.llvm.org/D47757#1211276, @tra wrote: > I've confirmed that the patch does not break anything in our CUDA code, so > it's good to go as far as CUDA is concerned. Thanks. @rsmith, do you have any other comments about the patch? Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
tra added a comment. I've confirmed that the patch does not break anything in our CUDA code, so it's good to go as far as CUDA is concerned. I'll fix the exposed CUDA issue in a separate patch. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
tra added a comment. Talked to @ahatanak over IRC. It appears that this patch may have exposed a preexisting bug. Apparently `delete t;` in test/SemaCUDA/call-host-fn-from-device.cu does actually end up calling `__host__ operator delete`. It should've picked `__device__ operator delete`, but it does not, so reporting an error here appears to be correct. It's visible in AST and the IR. @rsmith -- the original change was done a while back in https://reviews.llvm.org/rL283830. I assume it worked at that time and wonder if it's a (possibly not-so-)recent regression. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. The code you showed does compile with or without `-fcuda-is-device` after applying my patch. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
tra added a comment.
In https://reviews.llvm.org/D47757#1204621, @ahatanak wrote:
> In https://reviews.llvm.org/D47757#1204561, @tra wrote:
>
> > It's a regression. There's a decent chance it breaks someone and this
> > patch, if committed by itself, will end up being rolled back.
>
>
> Is the regression you are referring to about the static function case? I
> don't see a difference between ToT clang and my patch in the diagnostics they
> produce when I compile the following code:
>
> __host__ void f();
> static __host__ __device__ void g() { f(); }
> __host__ __device__ void g2() { g(); }
>
>
> Both error out when `-fcuda-is-device` is provided. If I comment out the
> definition of g2, it compiles fine.
The example above *is* expected to produce the error on device side, bacause
g2() is externally visible, uses g(), which in turn uses host-only f().
I'm talking about a case where g() {f()} is present in the source code, but
will not be codegen'ed on device side.
The code below is expected to compile. Note that g2() is host-only.
__host__ void f();
static __host__ __device__ void g() { f(); }
__host__ void g2() { g(); }
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment.
In https://reviews.llvm.org/D47757#1204561, @tra wrote:
> It's a regression. There's a decent chance it breaks someone and this patch,
> if committed by itself, will end up being rolled back.
Is the regression you are referring to about the static function case? I don't
see a difference between ToT clang and my patch in the diagnostics they produce
when I compile the following code:
__host__ void f();
static __host__ __device__ void g() { f(); }
__host__ __device__ void g2() { g(); }
Both error out when `-fcuda-is-device` is provided. If I comment out the
definition of g2, it compiles fine.
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
tra added a comment. In https://reviews.llvm.org/D47757#1204545, @ahatanak wrote: > @tra and @rsmith: Can we move forward and fix the incorrect cuda diagnostics > in a separate patch? Doing that in a separate patch is OK, provided that that patch will be committed along with this one. It's a regression. There's a decent chance it breaks someone and this patch, if committed by itself, will end up being rolled back. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. @tra and @rsmith: Can we move forward and fix the incorrect cuda diagnostics in a separate patch? Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
tra added inline comments.
Comment at: test/SemaCUDA/call-host-fn-from-device.cu:88
__host__ __device__ void class_specific_delete(T *t, U *u) {
- delete t; // ok, call sized device delete even though host has preferable
non-sized version
+ delete t; // expected-error {{reference to __host__ function 'operator
delete' in __host__ __device__ function}}
delete u; // ok, call non-sized HD delete rather than sized D delete
rsmith wrote:
> rsmith wrote:
> > tra wrote:
> > > The C++ magic is way above my paygrade, but as far as CUDA goes this is a
> > > regression, compared to what nvcc does. This code in NVCC produced a
> > > warning and clang should not error out at this point in time either as
> > > it's not an error to call a host function from HD unless we use HD in a
> > > host function, and we would not know how it's used until later. I think
> > > the error should be postponed until codegen.
> > >
> > >
> > >
> > >
> > We're in `-fcuda-is-device` mode, so IIUC it's correct to reject a call to
> > a host function here (because `__host__ __device__` is treated as basically
> > meaning `__device__` in that mode for the purpose of checking whether a
> > call is valid), right?
> >
> > However, the comment suggests that the intent was that this would instead
> > call the device version. Did that actually previously happen (in which case
> > this patch is somehow affecting overload resolution and should be fixed),
> > or is the comment prior to this patch wrong and we were silently calling a
> > host function from a device function (in which case this patch is fine, but
> > we should add a FIXME here to select the device delete function if we think
> > that's appropriate)?
> OK, I see from prior review comments (that phab is helpfully hiding from
> view) that this is just adding a diagnostic and the overload resolution
> behavior is unchanged. So I think this change is correct. @tra, can you
> confirm? My testing shows that
>
> ```
> __host__ void f(); __host__ __device__ void g() { f(); }
> ```
>
> is accepted by default but rejected in `-fcuda-is-device` mode, which is
> consistent with the behavior after this patch is applied.
"-fcuda-is-device" does not necessarily mean that the __host__ __device__
function will be used.
In the example above the error is indeed correct, as g() is considered to be
externally visible and we will attempt to generate code for it, and we can't
call f() on device.
However, if you make it static, there should be no error:
```
__host__ void f();
static __host__ __device__ void g() { f(); }
```
CUDA is somewhat weird when it comes to what's considered available and what is
not.
If you want some of the gory details, see D12453 and https://goo.gl/EXnymm
@jlebar has details on how we handle the errors in cases like that:
https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaCUDA.cpp#L590
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak updated this revision to Diff 160046.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Call MarkFunctionReferenced to mark the function `__builtin_operator_new` or
`__builtin_operator_delete` will call as referenced.
Repository:
rC Clang
https://reviews.llvm.org/D47757
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
test/CXX/drs/dr2xx.cpp
test/SemaCUDA/call-host-fn-from-device.cu
test/SemaCXX/unavailable_aligned_allocation.cpp
Index: test/SemaCXX/unavailable_aligned_allocation.cpp
===
--- test/SemaCXX/unavailable_aligned_allocation.cpp
+++ test/SemaCXX/unavailable_aligned_allocation.cpp
@@ -124,7 +124,73 @@
// expected-note@-20 2 {{if you supply your own aligned allocation functions}}
#endif
-// No errors if user-defined aligned allocation functions are available.
+// Test that diagnostics are produced when an unavailable aligned deallocation
+// function is called from a deleting destructor.
+struct alignas(256) OveralignedS2 {
+ int a[4];
+ virtual ~OveralignedS2();
+};
+
+OveralignedS2::~OveralignedS2() {}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+#if defined(IOS)
+// expected-error@-6 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on iOS 11 or newer}}}
+// expected-note@-7 {{if you supply your own aligned allocation functions}}
+#elif defined(TVOS)
+// expected-error@-9 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on tvOS 11 or newer}}}
+// expected-note@-10 {{if you supply your own aligned allocation functions}}
+#elif defined(WATCHOS)
+// expected-error@-12 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on watchOS 4 or newer}}}
+// expected-note@-13 {{if you supply your own aligned allocation functions}}
+#else
+// expected-error@-15 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on macOS 10.13 or newer}}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+#endif
+#endif
+
+void testExplicitOperatorNewDelete() {
+ void *p = operator new(128);
+ operator delete(p);
+ p = operator new[](128);
+ operator delete[](p);
+ p = __builtin_operator_new(128);
+ __builtin_operator_delete(p);
+}
+
+void testExplicitOperatorNewDeleteOveraligned() {
+ void *p = operator new(128, (std::align_val_t)64);
+ operator delete(p, (std::align_val_t)64);
+ p = operator new[](128, (std::align_val_t)64);
+ operator delete[](p, (std::align_val_t)64);
+ p = __builtin_operator_new(128, (std::align_val_t)64);
+ __builtin_operator_delete(p, (std::align_val_t)64);
+}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+// expected-error@-11 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-12 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-13 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-14 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-15 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-17 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-18 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-19 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-20 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-21 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-22 {{if you supply your own aligned allocation functions}}
+#endif
+
void *operator new(std::size_t __sz, std::align_val_t) {
static char array[256];
return &array;
Index: test/SemaCUDA/call-host-fn-from-device.cu
===
--- test/SemaCUDA/call-host-fn-from-device.cu
+++ test/SemaCUDA/call-host-fn-from-device.cu
@@ -85,7 +85,7 @@
}
__host__ __device__ void class_specific_delete(T *t, U *u) {
- delete t; // ok, call sized device delete even though host has preferable non-sized version
+ delete t; // expected-error {{reference to __host__ function 'operator delete' in __host__ __device__ function}}
delete u; // ok, call non-sized HD delete rather than sized D delete
}
Index: test/CXX/drs/dr2xx.cpp
===
--- test/CXX/drs/dr2xx.c
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
rsmith added inline comments.
Comment at: test/SemaCUDA/call-host-fn-from-device.cu:88
__host__ __device__ void class_specific_delete(T *t, U *u) {
- delete t; // ok, call sized device delete even though host has preferable
non-sized version
+ delete t; // expected-error {{reference to __host__ function 'operator
delete' in __host__ __device__ function}}
delete u; // ok, call non-sized HD delete rather than sized D delete
rsmith wrote:
> tra wrote:
> > The C++ magic is way above my paygrade, but as far as CUDA goes this is a
> > regression, compared to what nvcc does. This code in NVCC produced a
> > warning and clang should not error out at this point in time either as
> > it's not an error to call a host function from HD unless we use HD in a
> > host function, and we would not know how it's used until later. I think the
> > error should be postponed until codegen.
> >
> >
> >
> >
> We're in `-fcuda-is-device` mode, so IIUC it's correct to reject a call to a
> host function here (because `__host__ __device__` is treated as basically
> meaning `__device__` in that mode for the purpose of checking whether a call
> is valid), right?
>
> However, the comment suggests that the intent was that this would instead
> call the device version. Did that actually previously happen (in which case
> this patch is somehow affecting overload resolution and should be fixed), or
> is the comment prior to this patch wrong and we were silently calling a host
> function from a device function (in which case this patch is fine, but we
> should add a FIXME here to select the device delete function if we think
> that's appropriate)?
OK, I see from prior review comments (that phab is helpfully hiding from view)
that this is just adding a diagnostic and the overload resolution behavior is
unchanged. So I think this change is correct. @tra, can you confirm? My testing
shows that
```
__host__ void f(); __host__ __device__ void g() { f(); }
```
is accepted by default but rejected in `-fcuda-is-device` mode, which is
consistent with the behavior after this patch is applied.
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
rsmith added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:3463
+ DiagnoseUseOfDecl(OperatorNewOrDelete, TheCall->getExprLoc());
+
Are we also missing a `MarkFunctionReferenced` call here? (I don't think it
matters much for the predefined new/delete, since they can't be inline or
templated, but it's still wrong to not mark the function the builtin will call
as referenced.)
Comment at: test/CXX/drs/dr2xx.cpp:721-722
// FIXME: These are ill-formed, with a required diagnostic, for the same
// reason.
struct B {
These are -> This is
Comment at: test/SemaCUDA/call-host-fn-from-device.cu:88
__host__ __device__ void class_specific_delete(T *t, U *u) {
- delete t; // ok, call sized device delete even though host has preferable
non-sized version
+ delete t; // expected-error {{reference to __host__ function 'operator
delete' in __host__ __device__ function}}
delete u; // ok, call non-sized HD delete rather than sized D delete
tra wrote:
> The C++ magic is way above my paygrade, but as far as CUDA goes this is a
> regression, compared to what nvcc does. This code in NVCC produced a warning
> and clang should not error out at this point in time either as
> it's not an error to call a host function from HD unless we use HD in a host
> function, and we would not know how it's used until later. I think the error
> should be postponed until codegen.
>
>
>
>
We're in `-fcuda-is-device` mode, so IIUC it's correct to reject a call to a
host function here (because `__host__ __device__` is treated as basically
meaning `__device__` in that mode for the purpose of checking whether a call is
valid), right?
However, the comment suggests that the intent was that this would instead call
the device version. Did that actually previously happen (in which case this
patch is somehow affecting overload resolution and should be fixed), or is the
comment prior to this patch wrong and we were silently calling a host function
from a device function (in which case this patch is fine, but we should add a
FIXME here to select the device delete function if we think that's appropriate)?
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
tra added inline comments.
Comment at: test/SemaCUDA/call-host-fn-from-device.cu:88
__host__ __device__ void class_specific_delete(T *t, U *u) {
- delete t; // ok, call sized device delete even though host has preferable
non-sized version
+ delete t; // expected-error {{reference to __host__ function 'operator
delete' in __host__ __device__ function}}
delete u; // ok, call non-sized HD delete rather than sized D delete
The C++ magic is way above my paygrade, but as far as CUDA goes this is a
regression, compared to what nvcc does. This code in NVCC produced a warning
and clang should not error out at this point in time either as
it's not an error to call a host function from HD unless we use HD in a host
function, and we would not know how it's used until later. I think the error
should be postponed until codegen.
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
jlebar added a comment. +tra in the hopes that perhaps he's comfortable reviewing this (sorry that I'm not). Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. ping Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. ping Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. ping Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. Herald added a subscriber: dexonsmith. ping Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. Yes, that is the case. It doesn't change overloading resolution, it is just producing a diagnostic. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
jlebar added a comment. In https://reviews.llvm.org/D47757#1142886, @ahatanak wrote: > I mean ToT clang (without my patch applied) seems to select the non-sized > host version 'T::operator delete(void*)'. OK, if this is just making an error out of something which previously silently didn't work (and should result in a compile error further down the line when we try to and can't resolve that function), then this is totally fine. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment. I mean ToT clang (without my patch applied) seems to select the non-sized host version 'T::operator delete(void*)'. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a comment.
The original comment said "call sized device delete even though host has
preferable non-sized version", but it seems that the non-sized host version
'T::operator delete(void*)' is being called in the IR, not the sized device
version of delete. Is that a bug in overload resolution?
This is the command I used:
"clang -cc1 -internal-isystem --std=c++11 -triple nvptx-unknown-unknown
-fcuda-is-device-emit-llvm -o - test.cu"
$ cat test.cu
#define __constant__ __attribute__((constant))
#define __device__ __attribute__((device))
#define __host__ __attribute__((host))
typedef __SIZE_TYPE__ size_t;
struct T {
__host__ void operator delete(void*);
__device__ void operator delete(void*, size_t);
};
__host__ __device__ void class_specific_delete(T *t) {
delete t;
}
Repository:
rC Clang
https://reviews.llvm.org/D47757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
jlebar added a comment. > @jlebar, is the change I made to call-host-fn-from-device.cu correct? I don't think so -- that's a change in overloading behavior afaict. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak added a reviewer: jlebar. ahatanak added a subscriber: jlebar. ahatanak added a comment. ping. @jlebar, is the change I made to call-host-fn-from-device.cu correct? Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called
ahatanak updated this revision to Diff 150211.
ahatanak marked an inline comment as done.
ahatanak retitled this revision from "[Sema] Diagnose unavailable aligned
deallocation functions called from deleting destructors." to "[Sema] Produce
diagnostics when unavailable aligned allocation/deallocation functions are
called".
ahatanak added a comment.
Sink diagnoseUnavailableAlignedAllocation into DiagnoseUseOfDecl and add calls
to it in a few other places so that diagnostics are produced for calls to
aligned operator and builtin operator new/delete in addition to aligned
deallocation functions called from deleting destructors.
I had to make changes to two test cases (dr2xx.cpp and
call-host-fn-from-device.cu) that have nothing to do with aligned
allocation/deallocation functions. The warning in dr2xx.cpp seems correct to me
judging from the comment left few lines above it ("We're also missing the
-Wused-but-marked-unused "). I'm not sure about the diagnostic in
call-host-fn-from-device.cu. The original comment says the sized delete
function annotated with `__device__` is called, but it seems that the non-sized
`__host__` function is being called, in which case I think the diagnostic is
correct.
Repository:
rC Clang
https://reviews.llvm.org/D47757
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
test/CXX/drs/dr2xx.cpp
test/SemaCUDA/call-host-fn-from-device.cu
test/SemaCXX/unavailable_aligned_allocation.cpp
Index: test/SemaCXX/unavailable_aligned_allocation.cpp
===
--- test/SemaCXX/unavailable_aligned_allocation.cpp
+++ test/SemaCXX/unavailable_aligned_allocation.cpp
@@ -124,7 +124,73 @@
// expected-note@-20 2 {{if you supply your own aligned allocation functions}}
#endif
-// No errors if user-defined aligned allocation functions are available.
+// Test that diagnostics are produced when an unavailable aligned deallocation
+// function is called from a deleting destructor.
+struct alignas(256) OveralignedS2 {
+ int a[4];
+ virtual ~OveralignedS2();
+};
+
+OveralignedS2::~OveralignedS2() {}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+#if defined(IOS)
+// expected-error@-6 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on iOS 11 or newer}}}
+// expected-note@-7 {{if you supply your own aligned allocation functions}}
+#elif defined(TVOS)
+// expected-error@-9 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on tvOS 11 or newer}}}
+// expected-note@-10 {{if you supply your own aligned allocation functions}}
+#elif defined(WATCHOS)
+// expected-error@-12 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on watchOS 4 or newer}}}
+// expected-note@-13 {{if you supply your own aligned allocation functions}}
+#else
+// expected-error@-15 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on macOS 10.13 or newer}}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+#endif
+#endif
+
+void testExplicitOperatorNewDelete() {
+ void *p = operator new(128);
+ operator delete(p);
+ p = operator new[](128);
+ operator delete[](p);
+ p = __builtin_operator_new(128);
+ __builtin_operator_delete(p);
+}
+
+void testExplicitOperatorNewDeleteOveraligned() {
+ void *p = operator new(128, (std::align_val_t)64);
+ operator delete(p, (std::align_val_t)64);
+ p = operator new[](128, (std::align_val_t)64);
+ operator delete[](p, (std::align_val_t)64);
+ p = __builtin_operator_new(128, (std::align_val_t)64);
+ __builtin_operator_delete(p, (std::align_val_t)64);
+}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+// expected-error@-11 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-12 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-13 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-14 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-15 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-16 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-17 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is only available on}}
+// expected-note@-18 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-19 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' is only available on}}
+// expected-note@-20 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-21 {{aligned deallocation
