hnrklssn added inline comments.
================
Comment at:
clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12
+// CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT: ret i32 [[TMP1]]
+//
----------------
hnrklssn wrote:
> nikic wrote:
> > hnrklssn wrote:
> > > delcypher wrote:
> > > > @hnrklssn I just noticed we don't have a `CHECK` for what `META2`
> > > > actually refers to. Should we?
> > > >
> > > > Not something that has to be fixed in this patch, more just an
> > > > observation.
> > > Indeed this is true for metadata in general, presumably because the RHS
> > > often refer to things like other metadata identifiers. In the case of
> > > annotations they seem to always refer to simple strings however, so it
> > > would be feasible to do a straight match without having to do recursive
> > > matching or complex regexes to determine which part of the metadata to
> > > match against.
> > >
> > > In many cases with metadata attached to IR nodes, multiple nodes refer to
> > > the same metadata node, so at least you verify that they still are
> > > consistent. But I agree that verifying the content would be a great
> > > future addition.
> > You need to pass `--check-globals` to check the actual metadata.
> When I add that to this test case it adds
>
> ```
> //.
> // CHECK: attributes #0 = { noinline nounwind optnone
> "min-legal-vector-width"="0" "no-trapping-math"="true"
> "stack-protector-buffer-size"="8"
> "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
> //.
> // CHECK: !0 = !{i32 1, !"wchar_size", i32 4}
> // CHECK: !1 = !{!"clang version 17.0.0
> (g...@github.com:llvm/llvm-project.git
> 684914f47cf59e9ab6d8b0f73c58ca6272ea28d4)"}
> // CHECK: !2 = !{!"auto-init"}
> //.
> ```
>
> So it seems to just be doing a simple literal matching on all metadata,
> regardless of whether we captured that metadata in any filecheck variable.
> And it still doesn't use the META2 variable to match the definition. Am I
> missing something? If we use the literal metadata names instead of variable
> matching for the definitions, there isn't much point in doing variable
> matching for the metadata uses either, since the test still very much relies
> on the metadata numbering being stable.
@nikic Do you have more information to add about how metadata definition
matchers can be generated without hardcoding everything (which is kind of the
opposite of what this patch is trying to do), or in general if you're happy
with the state of the PR?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148216/new/
https://reviews.llvm.org/D148216
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
