<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Jeroen,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks for reporting this! Just FYI I believe the commit sha we are discussing is f041e9ad706aee7987c5299427c33424fcabbd0d.<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
This seems very much related to the open bug regarding nested pointer conversion with address spaces:
<a href="http://llvm.org/PR39674" title="llvm.org/PR39674">llvm.org/PR</a><a id="LPlnk428016">39674</a>.<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Indeed address spaces (unlike other qualifiers in pure C++ mode) do modify the type representation. So we can't safely drop/change them without special adjustment operations that aren't feasible/trivial for the nested pointer case. In fact looking at the Richard's
 commit message he has already highlighted the fact that the nested pointer behavior currently isn't correct: "<span>This makes the behavior of reference</span> binding consistent with that of implicit pointer conversions, as is the purpose of this change,
 but that pre-existing behavior for pointer <span>conversions is itself probably not correct.</span>"</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
It should be relatively straight forward to reinstate the old behavior by recovering previous logic that checks address spaces. Then they will just be handled differently to regular C++ qualifiers like for example some of ObjC++ qualifiers already do. If there
 are no objections I am happy to work on the patch to fix the issue.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Cheers,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Anastasia<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="appendonsend"></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Jeroen Dobbelaere <Jeroen.Dobbelaere@synopsys.com><br>
<b>Sent:</b> 10 January 2020 14:20<br>
<b>To:</b> richard@metafoo.co.uk <richard@metafoo.co.uk>; clang Development List (cfe-dev@lists.llvm.org) <cfe-dev@lists.llvm.org><br>
<b>Cc:</b> Anastasia Stulova <Anastasia.Stulova@arm.com><br>
<b>Subject:</b> Problem with address spaces and 'CWG2352: Allow qualification conversions during reference binding.'</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">Hi Richard,<br>
<br>
I have found some issues with following patch: <br>
   346de9b67228f42eb9b55fa3b426b5dedfdb1d40   CWG2352: Allow qualification conversions during reference binding.<br>
<br>
There are two unwanted effects that we observe now (see testcase below):<br>
1) It has effect on the implicit promotions of 'pointers pointing to an address space'. Code that would (should) give an error is now silently accepted.
<br>
<br>
2) Code that worked, now triggers an internal assertion error.<br>
clang-10: /..../llvm-project/llvm/include/llvm/IR/Instructions.h:2656: void llvm::PHINode::setIncomingValue(unsigned int, llvm::Value*): Assertion `getType() == V->getType() && "All operands to PHI node must be the same type as the PHI node!"' failed.                                                                                                                                                                                                                                                
<br>
<br>
about the 'FIXME: Does "superset" also imply the representation of a pointer is the same?' comment in the patch:<br>
    -> no, even if an address space is a superset of another, their representations can differ. It is only after converting<br>
        the clang AS to the target AS that you have some more information about the representation:<br>
        for example, for x86, the different opencl address spaces (__global, __private, __generic, ...) seem to map onto the<br>
        same pointer type (target AS 0). For amdgcn, this is not the case.<br>
<br>
NOTE: I added Anastasia, as she is also interested in everything related to address spaces.<br>
<br>
Greetings,<br>
<br>
Jeroen Dobbelaere<br>
<br>
----- test.c -----<br>
// use an amdgcn target as that has separate target address spaces<br>
// clang  -target amdgcn-unknown-amdhsa -x cl -cl-std=clc++ -O3 -S -emit-llvm -Xclang -disable-llvm-passes -o - test.c<br>
#define SUP __generic <br>
#define SUB __global <br>
<br>
#if 1<br>
  extern void foo(int SUP*&);<br>
  int test_should_give_error(int SUB*p) {<br>
    foo(p); // ISSUE1: int SUB* cannot be passes as a 'int SUP* &'  -> should give an error<br>
    return *p;<br>
  }<br>
#endif<br>
<br>
  extern void foo_constant(int SUP* const &);<br>
  int test_pass_constant(int SUB*p) {<br>
    foo_constant(p); // int SUB* is first converted into a local int SUP*, and then passed as a 'int SUP* const&' -> ok<br>
    return *p;<br>
  }<br>
<br>
int test6(int c, int SUB* a, int SUP* b) {<br>
    return *(c ? a : b); // ISSUE2: internal error -> should return something like: '*(c ? (int SUP*)a : b)'<br>
}<br>
----- test.c --<br>
  <br>
</div>
</span></font></div>
</body>
</html>