[PATCH] Use attachLowHighPC also for compile unit DIE

David Blaikie dblaikie at gmail.com
Thu Aug 28 13:58:21 PDT 2014


Looks good to me - please commit

A few thoughts:

* low-pc-cu.ll creates temporary files, rather than piping the llc output
straight to dwarfdump - if you want to cleanup the existing use to pipe
directly, and make your new use do the same, that'd be nice (can be done in
the same commit, or cleanup first then following the newer convention with
your main commit) * if FileCheck requires a CHECKPREFIX before a
CHECKPREFIX-NEXT, that seems like a bug to me (but I could be wrong) - it
seems perfectly reasonable (as in your case) that we might have a CHECK1
using one prefix, then a CHECK2-NEXT using a different prefix. If you're
curious, perhaps you could look into fixing the FileCheck bug there? But
it's not important in this case, because... * to make tests more resilient
to attribute ordering changes, I usually avoid using CHECK-NEXT, instead I
put a CHECK-NOT: DW_TAG between a tag and its attribute and between each
attribute - that way other attributes can appear and they don't break the
test, but we don't accidentally end up checking the attributes of some
other tag. (and I use CHECK-NOT: {{DW_TAG|NULL}} between one tag and the
next to make sure there's no extra tags in between (especially important if
they change the parenting relationship - eg: if I want to find a child of
this tag, I don't necessarily want to succeed if the node I'm looking for
is a grandchild rather than a direct child))


On Thu, Aug 28, 2014 at 1:39 PM, Frédéric Riss <friss at apple.com> wrote:

>
> On 28 Aug 2014, at 20:13, Frédéric Riss <friss at apple.com> wrote:
>
>
> On 28 Aug 2014, at 20:01, David Blaikie <dblaikie at gmail.com> wrote:
>
> Do we have existing test coverage for the subprogram case? I imagine we do
> somewhere (try breaking it and see which tests fail) though it might not be
> a nice/tidy as your test case. Perhaps you could add the CU coverage to the
> same test case (& possibly clean it up a bit if it's not as legible as your
> new case)
>
>
> I can try to do that.
>
>
> Here is a new patch piggybacking the testing onto 2 existing tests. This
> way we verify compile_unit, subprogram and lexical_scope encodings.
>
> In the lexical scope test, I dropped the -NEXT constraint on the pattern
> as it would require duplicating the whole CHECK patterns (a CHECK-V4-NEXT
> needs a previous CHECK-V4). I wasn’t sure which way is better, I can of
> course keep all the -NEXT constraints and duplicate the whole pattern 2
> times.
>
> Fred
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140828/88162438/attachment.html>


More information about the llvm-commits mailing list