[PATCH] D92751: [clang][aarch64] Precondition isHomogeneousAggregate on isCXX14Aggregate

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm added a comment.

Suggesting a few small tweaks. Otherwise LGTM when @rnk is happy.



================
Comment at: clang/test/CodeGenCXX/homogeneous-aggregates.cpp:129
+struct Empty {};
+// A class with a base is returned using the sret calling convetion by MSVC.
+struct HasEmptyBase : public Empty {
----------------



================
Comment at: clang/test/CodeGenCXX/homogeneous-aggregates.cpp:147
+  *pod = copy(pod);
+  // WOA64-LABEL: %{{.*}} = call %"struct.pr47611::Pod" @"?copy at pr47611@@YA?AUPod at 1@PEAU21@@Z"(%"struct.pr47611::Pod"* %{{.*}})
+}
----------------
Same comment (from below) about -LABEL for each of these functions. This should be a 'check'-alike and each function header should have a CHECK-LABEL.


================
Comment at: llvm/test/CodeGen/AArch64/arm64-windows-calls.ll:117
+  ret %struct.Pod %x1
+; CHECK: ldp d0, d1, [x0]
+}
----------------
rnk wrote:
> Please use CHECK-LABEL directives to ensure that these assembly checks are from the function you intend to match.
@DavidTruby I think the intent here is:

CHECK-LABEL: < something matching the first line of the function definition >
CHECK: <thing to be checked>

CHECK-LABEL: <next function>

Such that if by some fluke some other function in the interim produces <thing to be checked>, it won't be allowed to match something produced in the subsequent function. This also renders the code safe against this problem if functions are added or moved around. I suggest having a CHECK-LABEL on each function heading, and the code within those functions should be CHECK/CHECK-NEXT as appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92751