Anastasia marked an inline comment as done.
Anastasia added inline comments.
================
Comment at: lib/CodeGen/CGCall.cpp:4067
+ IRFuncTy->getParamType(FirstIRArg)->isPointerTy())
+ V = Builder.CreatePointerBitCastOrAddrSpaceCast(
+ V, IRFuncTy->getParamType(FirstIRArg));
----------------
rjmccall wrote:
> Anastasia wrote:
> > We have started using `performAddrSpaceCast` for those but, however, I was
> > very confused about how to get address space of the Clang types correctly
> > here.
> >
> > I was using this function in a couple of places before but I must admit I
> > am still a bit confused about the purpose of it. Mainly I was wondering if
> > we could drop `LangAS` parameters from it? It would simplify its use a lot
> > in multiple places. As far as I can see they are not used in the function
> > at the moment. I am not sure if they are reserved for some development in
> > the future though?
> >
> > Altogether I quite like using `IRBuilder` directly because in combination
> > with target address space map it allows to do what's needed here and keep
> > consistent with the rest. Perhaps, I am missing some background info. I
> > have tried to dig out a bit but no much success.
> We want to allow language address spaces to be arbitrarily mapped to IR
> address spaces during lowering, which might include e.g. changing the null
> value. That's why we pass language address spaces down.
>
> You should be getting the language address space from the appropriate type.
> I wouldn't expect that to include address-space conversions at this point,
> though; is there maybe a missing implicit conversions somewhere?
> We want to allow language address spaces to be arbitrarily mapped to IR
> address spaces during lowering, which might include e.g. changing the null
> value. That's why we pass language address spaces down.
Would this mean we can map AST address spaces into different IR ones from what
is the the target address space map? Ok right, null pointer value can't be
handled by the target address space map at all. I am a bit confused though why
we are not using these LangAS at the moment. I guess we just don't have
upstream targets requiring that...
> is there maybe a missing implicit conversions somewhere?
Ok, the AST dump doesn't seem right for the test case I included.
`MaterializeTemporaryExpr` is in `generic` addr space. I guess it should be
`private`... and then converted to `generic`.
```
`-FunctionDecl 0x712608 <line:5:1, line:11:1> line:5:6 foo 'void ()'
`-CompoundStmt 0x712810 <col:11, line:11:1>
`-ExprWithCleanups 0x7127f8 <line:10:3, col:8> 'int'
`-CallExpr 0x7127a0 <col:3, col:8> 'int'
|-ImplicitCastExpr 0x712788 <col:3> 'int (*)(const __generic unsigned
int &)' <FunctionToPointerDecay>
| `-DeclRefExpr 0x712710 <col:3> 'int (const __generic unsigned int &)'
lvalue Function 0x7124d0 'bar' 'int (const __generic unsigned int &)'
`-MaterializeTemporaryExpr 0x7127e0 <col:7> 'const __generic unsigned
int' lvalue
`-ImplicitCastExpr 0x7127c8 <col:7> 'const __generic unsigned int'
<IntegralCast>
`-IntegerLiteral 0x7126f0 <col:7> 'int' 1
```
I will fix it!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58634/new/
https://reviews.llvm.org/D58634
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
