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

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 11 12:14:57 PST 2021


rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks



================
Comment at: llvm/test/CodeGen/AArch64/arm64-windows-calls.ll:117
+  ret %struct.Pod %x1
+; CHECK: ldp d0, d1, [x0]
+}
----------------
peterwaller-arm wrote:
> 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.
Yep. I think you could still go further by adding a trailing colon. FileCheck matches for substrings of a line, so as written, this could match "call copy_pod", which wouldn't be what you want. The risk of that in this test is low, but it's a good best practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92751



More information about the cfe-commits mailing list