[cfe-dev] Problem with address spaces and 'CWG2352: Allow qualification conversions during reference binding.'

[Anastasia Stulova via cfe-dev]

Hi Jeroen,

Thanks for reporting this! Just FYI I believe the commit sha we are discussing is f041e9ad706aee7987c5299427c33424fcabbd0d.

This seems very much related to the open bug regarding nested pointer conversion with address spaces: llvm.org/PR<http://llvm.org/PR39674>39674.

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: "This makes the behavior of reference binding consistent with that of implicit pointer conversions, as is the purpose of this change, but that pre-existing behavior for pointer conversions is itself probably not correct."

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.

Cheers,
Anastasia


________________________________
From: Jeroen Dobbelaere <Jeroen.Dobbelaere at synopsys.com>
Sent: 10 January 2020 14:20
To: richard at metafoo.co.uk <richard at metafoo.co.uk>; clang Development List (cfe-dev at lists.llvm.org) <cfe-dev at lists.llvm.org>
Cc: Anastasia Stulova <Anastasia.Stulova at arm.com>
Subject: Problem with address spaces and 'CWG2352: Allow qualification conversions during reference binding.'

Hi Richard,

I have found some issues with following patch:
   346de9b67228f42eb9b55fa3b426b5dedfdb1d40   CWG2352: Allow qualification conversions during reference binding.

There are two unwanted effects that we observe now (see testcase below):
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.

2) Code that worked, now triggers an internal assertion error.
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.

about the 'FIXME: Does "superset" also imply the representation of a pointer is the same?' comment in the patch:
    -> no, even if an address space is a superset of another, their representations can differ. It is only after converting
        the clang AS to the target AS that you have some more information about the representation:
        for example, for x86, the different opencl address spaces (__global, __private, __generic, ...) seem to map onto the
        same pointer type (target AS 0). For amdgcn, this is not the case.

NOTE: I added Anastasia, as she is also interested in everything related to address spaces.

Greetings,

Jeroen Dobbelaere

----- test.c -----
// use an amdgcn target as that has separate target address spaces
// clang  -target amdgcn-unknown-amdhsa -x cl -cl-std=clc++ -O3 -S -emit-llvm -Xclang -disable-llvm-passes -o - test.c
#define SUP __generic
#define SUB __global

#if 1
  extern void foo(int SUP*&);
  int test_should_give_error(int SUB*p) {
    foo(p); // ISSUE1: int SUB* cannot be passes as a 'int SUP* &'  -> should give an error
    return *p;
  }
#endif

  extern void foo_constant(int SUP* const &);
  int test_pass_constant(int SUB*p) {
    foo_constant(p); // int SUB* is first converted into a local int SUP*, and then passed as a 'int SUP* const&' -> ok
    return *p;
  }

int test6(int c, int SUB* a, int SUP* b) {
    return *(c ? a : b); // ISSUE2: internal error -> should return something like: '*(c ? (int SUP*)a : b)'
}
----- test.c --

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200114/7dde3433/attachment-0001.html>