[PATCH] Use attachLowHighPC also for compile unit DIE

[Frédéric Riss]

> On 28 Aug 2014, at 22:58, David Blaikie <dblaikie at gmail.com> wrote:
> 
> Looks good to me - please commit

Thanks, this is r216719

> 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)
> 

Done in the same patch.

> * 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... 
> 

This in fact works. I forgot to specify —check-prefix=CHECK along with the —check-prefix=CHECK-V[34]...

> * 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))

... but I changed it for this more robust approach anyway.

Fred 

> 
> 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
> 
> 
>