[PATCH] D72271: [Clang] Handle target-specific builtins returning aggregates.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4333
+    if (!ReturnValue.isNull() &&
+        ReturnValue.getValue().getType()->getPointerElementType()->isStructTy())
+      return RValue::getAggregate(ReturnValue.getValue(),
----------------
The requirement is that we construct an `RValue` of the right kind for the expression's result type, so the correct approach here is to switch over `getEvaluationKind(E->getType())`.  This is also a more reliable signal than the presence of the return value slot.

We then need to think about how the return value will be returned to us in each case:

- For scalars, `RValue::get(V)` is right.

- We can assert for now that there are no target builtins returning a complex type; if we ever need to add this, we'll probably have to break apart an IR struct.

- So that just leaves aggregates.  I believe we can't rely on `ReturnValue` being non-null on entry to this function; for example, IIRC we'll pass down a null slot if the code does an immediate member access into the result, e.g. `vld2q(...).val[0]`.  It looks like the custom logic for VLD24 does not do anything reasonable in this case: it returns a first-class struct, which the rest of IRGen will definitely not expect.  Probably the most reasonable thing would be to create a slot if we have an aggregate result and the caller didn't give us a slot (out here in `EmitBuiltinExpr`), and then we can just require `EmitTargetBuiltinExpr` to return null or something else that makes it clear that they've done the right thing with the slot that was passed in.  You can create a slot with `CreateMemTemp`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72271/new/

https://reviews.llvm.org/D72271