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

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: dmgreen, rjmccall.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.

A few of the ARM MVE builtins directly return a structure type. This
causes an assertion failure at code-gen time if you try to assign the
result of the builtin to a variable, because the `RValue` created in
`EmitBuiltinExpr` from the `llvm::Value` produced by codegen is always
made by `RValue::get()`, which creates a non-aggregate `RValue` that
will fail an assertion when `AggExprEmitter::withReturnValueSlot`
calls `Src.getAggregatePointer()`.

To fix it, I've added a check in `EmitBuiltinExpr` so that it will
construct an aggregate `RValue` if the provided return value slot
contains a valid `Address` whose type is a pointer to struct. As far
as I can see that makes no difference to any existing builtin, and it
fixes the crash in the structure-typed MVE ones.

The existing code-gen tests for those MVE builtins pass the returned
structure type directly to the C `return` statement, which apparently
managed to avoid that particular code path, so we didn't notice the
crash. But as soon as you try to assign the returned structure into
(for example) a local variable, it does crash. Added an extra test
that reproduced the bug before this change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72271

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/arm-mve-intrinsics/vld24.c


Index: clang/test/CodeGen/arm-mve-intrinsics/vld24.c
===================================================================
--- clang/test/CodeGen/arm-mve-intrinsics/vld24.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vld24.c
@@ -98,3 +98,27 @@
     vst2q_f16(addr, value);
 #endif /* POLYMORPHIC */
 }
+
+// CHECK-LABEL: @load_via_variable(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = call { <8 x i16>, <8 x i16> } @llvm.arm.mve.vld2q.v8i16.p0i16(i16* [[ADDR:%.*]])
+// CHECK-NEXT:    [[TMP1:%.*]] = extractvalue { <8 x i16>, <8 x i16> } [[TMP0]], 0
+// CHECK-NEXT:    [[TMP2:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T:%.*]] undef, <8 x i16> [[TMP1]], 0, 0
+// CHECK-NEXT:    [[TMP3:%.*]] = extractvalue { <8 x i16>, <8 x i16> } [[TMP0]], 1
+// CHECK-NEXT:    [[TMP4:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T]] [[TMP2]], <8 x i16> [[TMP3]], 0, 1
+// CHECK-NEXT:    store <8 x i16> [[TMP1]], <8 x i16>* [[VALUES:%.*]], align 8
+// CHECK-NEXT:    [[ARRAYIDX4:%.*]] = getelementptr inbounds <8 x i16>, <8 x i16>* [[VALUES]], i32 1
+// CHECK-NEXT:    store <8 x i16> [[TMP3]], <8 x i16>* [[ARRAYIDX4]], align 8
+// CHECK-NEXT:    ret void
+//
+void load_via_variable(const uint16_t *addr, uint16x8_t *values)
+{
+    uint16x8x2_t v;
+#ifdef POLYMORPHIC
+    v = vld2q(addr);
+#else /* POLYMORPHIC */
+    v = vld2q_u16(addr);
+#endif /* POLYMORPHIC */
+    values[0] = v.val[0];
+    values[1] = v.val[1];
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -4328,8 +4328,13 @@
   }
 
   // See if we have a target specific builtin that needs to be lowered.
-  if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue))
+  if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue)) {
+    if (!ReturnValue.isNull() &&
+        ReturnValue.getValue().getType()->getPointerElementType()->isStructTy())
+      return RValue::getAggregate(ReturnValue.getValue(),
+                                  ReturnValue.isVolatile());
     return RValue::get(V);
+  }
 
   ErrorUnsupported(E, "builtin function");
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72271.236359.patch
Type: text/x-patch
Size: 2152 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200106/6e2fefe1/attachment.bin>