[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    [PATCH] D43842: CodeGenObjCXX: handle inalloca appropriately for msgSend variant
From:       Reid Kleckner via Phabricator via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2018-02-28 21:51:51
Message-ID: c242956c76e89c70ca8d528ebe3d74ac () localhost ! localdomain
[Download RAW message or body]

rnk added a comment.

In https://reviews.llvm.org/D43842#1022546, @rjmccall wrote:

> Okay.  In that case, this seems correct, although it seems to me that perhaps \
> `inalloca` is not actually orthogonal to anything else.  In fact, it seems to me \
> that maybe `inalloca` ought to just be a bit on the CGFunctionInfo and the \
> individual ABIInfos should be left alone.


I don't think this is a good idea, since when inalloca mixes with `__thiscall` and \
`__fastcall`, register parameters are still passed directly. Consider:

  struct S {
    S();
    S(const S &);
    int x;
    void thiscall_byval(S o);
  };
  void S::thiscall_byval(S o) {}
  int __fastcall fastcall_byval(int x, int y, S o) { return x + y; }
  
  $ clang -S t.cpp -emit-llvm -m32 -o - | grep define
  define dso_local x86_thiscallcc void @"\01?thiscall_byval@S@@QAEXU1@@Z"(%struct.S* \
%this, <{ %struct.S }>* inalloca) #0 align 2 {  define dso_local x86_fastcallcc i32 \
@"\01?fastcall_byval@@YIHHHUS@@@Z"(i32 inreg %x, i32 inreg %y, <{ %struct.S }>* \
inalloca) #0 {

So sometimes the sret parameter is not in the inalloca pack:

  S __fastcall fastcall_sret(int x, int y, S o) { return o; }
  define dso_local x86_fastcallcc void \
@"\01?fastcall_sret@@YI?AUS@@HHU1@@Z"(%struct.S* inreg noalias sret %agg.result, i32 \
inreg %x, <{ i32, %struct.S }>* inalloca)

I think the long term way to clean this up is to use separate allocations for each \
argument, and use tokens to prevent the middle end from messing up the SESE region \
between the argument allocation point and the call that consumes the allocations.

To call foo, the IR would look like:

  void foo(int x, S y, int z, S w);
  ..
  foo(1, S(2), 3, S(4));
  ...
  ; setup w
  %w.token = call token @llvm.allocarg(i32 4)
  %w.addr = call %S* @llvm.argaddr(token %w.token)
  call void @S_ctor(%S* %w.addr, i32 4)
  ; setup y
  %y.token = call token @llvm.allocarg(i32 4)
  %y.addr = call %S* @llvm.argaddr(token %y.token)
  call void @S_ctor(%S* %y.addr, i32 2)
  ; do the call, consume the tokens
  call void @foo(i32 1, %S* inalloca %y.addr, i32 3, %S* inalloca %w.addr) \
"allocargs"[token %w.token, token %y.token]

The tokens would make it illegal to tail merge two calls to foo in an if/else diamond \
with PHIs.

It would also mean that all of this Obj-C IRGen code can go back to assuming that \
pointers are passed directly, because now the presence of one non-trivially copyable \
struct doesn't affect the ABIInfo of every other argument.

I am concerned about what happens in evil code like:

    foo(({ goto abort_call; 0 }), S(2), 3, S(4));
  abort_call:

In this case, control flow breaks out of expression evaluation, and we never \
deallocate the stack memory for the call. We'd need some way to clean that up, \
perhaps with a cleanup.

This would all be a lot easier if LLVM IR had scopes and it wasn't all just basic \
block soup. Just saying.


Repository:
  rC Clang

https://reviews.llvm.org/D43842



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]