<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>From:</b> Richard Smith [mailto:richard@metafoo.co.uk] <br>
<b>Sent:</b> Friday, July 20, 2018 3:29 PM<br>
<b>To:</b> Liu, Yaxun (Sam) <Yaxun.Liu@amd.com><br>
<b>Cc:</b> cfe-commits <cfe-commits@lists.llvm.org><br>
<b>Subject:</b> Re: r337540 - Sema: Fix explicit address space cast in C++<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<p class="MsoNormal">On Fri, 20 Jul 2018 at 12:00, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<p class="MsoNormal">On Fri, 20 Jul 2018 at 04:38, Yaxun Liu via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">Author: yaxunl<br>
Date: Fri Jul 20 04:32:51 2018<br>
New Revision: 337540<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=337540&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=337540&view=rev</a><br>
Log:<br>
Sema: Fix explicit address space cast in C++<br>
<br>
Currently clang does not allow implicit cast of a pointer to a pointer type<br>
in different address space but allows C-style cast of a pointer to a pointer<br>
type in different address space. However, there is a bug in Sema causing<br>
incorrect Cast Expr in AST for the latter case, which in turn results in<br>
invalid LLVM IR in codegen.<br>
<br>
This is because Sema::IsQualificationConversion returns true for a cast of<br>
pointer to a pointer type in different address space, which in turn allows<br>
a standard conversion and results in a cast expression with no op in AST.<br>
<br>
This patch fixes that by let Sema::IsQualificationConversion returns false<br>
for a cast of pointer to a pointer type in different address space, which<br>
in turn disallows standard conversion, implicit cast, and static cast.<br>
Finally it results in an reinterpret cast and correct conversion kind is set.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D49294" target="_blank">
https://reviews.llvm.org/D49294</a><br>
<br>
Added:<br>
cfe/trunk/test/CodeGenCXX/address-space-cast.cpp<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaCast.cpp<br>
cfe/trunk/lib/Sema/SemaOverload.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaCast.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=337540&r1=337539&r2=337540&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=337540&r1=337539&r2=337540&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaCast.cpp Fri Jul 20 04:32:51 2018<br>
@@ -1955,6 +1955,12 @@ static bool fixOverloadedReinterpretCast<br>
return Result.isUsable();<br>
}<br>
<br>
+static bool IsAddressSpaceConversion(QualType SrcType, QualType DestType) {<br>
+ return SrcType->isPointerType() && DestType->isPointerType() &&<br>
+ SrcType->getAs<PointerType>()->getPointeeType().getAddressSpace() !=<br>
+ DestType->getAs<PointerType>()->getPointeeType().getAddressSpace();<br>
+}<br>
+<br>
static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,<br>
QualType DestType, bool CStyle,<br>
SourceRange OpRange,<br>
@@ -2198,6 +2204,8 @@ static TryCastResult TryReinterpretCast(<br>
} else {<br>
Kind = CK_BitCast;<br>
}<br>
+ } else if (IsAddressSpaceConversion(SrcType, DestType)) {<br>
+ Kind = CK_AddressSpaceConversion;<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This seems wrong to me. A reinterpret_cast from a pointer to one address space into a pointer to a different address space should be a bit cast, not an address space conversion.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Hmm. I suppose we have a choice: either reinterpret_cast always means 'reinterpret these bits as this other type' (and there are C-style casts that cannot be expressed in terms of named casts, violating the usual C++ rules for C-style casts)
or reinterpret_cast between pointers always gives you a pointer to the same byte of storage and an address space bitcast requires casting via an integral type (violating the normal behavior that reinterpret_cast reinterprets the representation and doesn't
change the value). These options both seem unsatisfying, but what you have here (reinterpret_cast attempts to form a pointer to the same byte) is definitely the more useful of those two options.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Do any of our supported language extensions actually say what named casts in C++ do when attempting to cast between address spaces? The OpenCL 2.2 C++ extensions specification doesn't add address space qualifiers, so it doesn't have an
answer for us.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[Sam] I don’t know C++ based languages having rules about pointers casting to a different address space. OpenCL C’s rules about C-style cast of pointers to a different address space clearly means pointing to the same memory location. Bitwise
cast isn’t useful in most use cases. Since there is no language rule in C++ for casting a pointer to a different address space, the C-style cast cannot be treated as a static cast in C++ since it mostly involves transformations, then it is more suitably treated
as a reinterpret cast. If users do need bitwise cast, they can go through some integer, as you suggested.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal"> } else {<br>
Kind = CK_BitCast;<br>
}<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=337540&r1=337539&r2=337540&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=337540&r1=337539&r2=337540&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Jul 20 04:32:51 2018<br>
@@ -3150,6 +3150,15 @@ Sema::IsQualificationConversion(QualType<br>
= PreviousToQualsIncludeConst && ToQuals.hasConst();<br>
}<br>
<br>
+ // Allows address space promotion by language rules implemented in<br>
+ // Type::Qualifiers::isAddressSpaceSupersetOf.<br>
+ Qualifiers FromQuals = FromType.getQualifiers();<br>
+ Qualifiers ToQuals = ToType.getQualifiers();<br>
+ if (!ToQuals.isAddressSpaceSupersetOf(FromQuals) &&<br>
+ !FromQuals.isAddressSpaceSupersetOf(ToQuals)) {<o:p></o:p></p>
</blockquote>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Is this right? Qualification conversions are usually only permitted for "guaranteed-safe" conversions, which would be cases where the target address space is a superset of the source address space only. And OpenCL 2.2's C extensions specification
(section 1.5.6, changes to 6.3.2.3) says:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> "For any qualifier q, a pointer to a non-q-qualified type may be converted to a pointer to the q-qualified version of the type (but with the same address-space qualifier or the generic address space); the values stored in the original
and converted pointers shall compare equal."<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">... which looks to me like OpenCL only permits going from subset to superset address space in C's equivalent to a qualification conversion. I don't think we want to support:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> char *p;<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> __private__ char *q = p;<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">(with no cast) in C++.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">+ return false;<br>
+ }<br>
+<br>
// We are left with FromType and ToType being the pointee types<br>
// after unwrapping the original FromType and ToType the same number<br>
// of types. If we unwrapped any pointers, and if FromType and<br>
<br>
Added: cfe/trunk/test/CodeGenCXX/address-space-cast.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/address-space-cast.cpp?rev=337540&view=auto" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/address-space-cast.cpp?rev=337540&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/address-space-cast.cpp (added)<br>
+++ cfe/trunk/test/CodeGenCXX/address-space-cast.cpp Fri Jul 20 04:32:51 2018<br>
@@ -0,0 +1,15 @@<br>
+// RUN: %clang_cc1 %s -triple=amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck %s<br>
+<br>
+#define __private__ __attribute__((address_space(5)))<br>
+<br>
+void func_pchar(__private__ char *x);<br>
+<br>
+void test_cast(char *gen_ptr) {<br>
+ // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*<br>
+ // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]<br>
+ __private__ char *priv_ptr = (__private__ char *)gen_ptr;<br>
+<br>
+ // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*<br>
+ // CHECK-NEXT: call void @_Z10func_pcharPU3AS5c(i8 addrspace(5)* %[[cast]])<br>
+ func_pchar((__private__ char *)gen_ptr);<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</body>
</html>