This is an archive of the discontinued LLVM Phabricator instance.

Read discriminators correctly from object file.
ClosedPublic

Authored by danielcdh on Apr 25 2016, 9:48 PM.

Details

Summary

This is the follow-up patch for http://reviews.llvm.org/D19436

  • Update the discriminator reading algorithm to match the assignment algorithm.
  • Add test to cover the new algorithm.

Diff Detail

Event Timeline

danielcdh updated this revision to Diff 54965.Apr 25 2016, 9:48 PM
danielcdh retitled this revision from to Read discriminators correctly from object file..
danielcdh updated this object.
danielcdh added reviewers: dblaikie, dnovillo, echristo.
danielcdh added a subscriber: llvm-commits.
echristo edited edge metadata.Apr 25 2016, 11:18 PM
echristo added a subscriber: echristo.

Formatting, a comment about the discriminator in the comment there and
lgtm.

danielcdh updated this revision to Diff 55008.Apr 26 2016, 8:28 AM
danielcdh edited edge metadata.

Add comment and reformat.

dblaikie edited edge metadata.Apr 26 2016, 10:32 AM
dblaikie added a subscriber: dblaikie.

Usually we try to reduce the source code before generating the IR test case
(looks like this IR may've been hand reduced further? But I may be
mistaken) for debug info tests & include the source in a comment at the top
of the test so it's easy to understand the basic intent of the test case -
since reconstructing the intent/semantics from the metadata is somewhat
non-trivial.

(also, sometimes we checkin binaries to test the dumper - we don't always,
and I'm not suggesting it here, just mentioning it as a thought - sometimes
we just treat the dumper as "golden" and assume we're just testing the LLVM
IR/CodeGen behavior)

  • Dave
danielcdh updated this revision to Diff 55040.Apr 26 2016, 10:50 AM
danielcdh edited edge metadata.

Add comment about the original source file.

dblaikie added inline comments.Apr 26 2016, 12:46 PM
test/DebugInfo/X86/discriminator2.ll
10

Would it be simpler/sufficient to do this:

void f1();
void f2() {
  f1(); f1();
  f1(); f1();
}

(with column info off? Or, to keep column info on, #define CALLS f1(); f1() and have the body of the function be:

CALLS;
CALLS;

)

Unfortunately, we can't because I need to have two consecutive
instructions that are both from discriminator 1, but different lines
to trigger this bug. With your proposed change, the code sequence will
be:

line 4 discriminator 0
line 4 discriminator 1
line 5 discriminator 0
line 5 discriminator 1

But with my test case, the code sequence will be:

line 4 discriminator 0
line 5 discriminator 0
line 4 discriminator 1
line 5 discriminator 1

danielcdh updated this revision to Diff 55303.Apr 27 2016, 1:18 PM

comment about the test.

danielcdh updated this revision to Diff 55367.Apr 27 2016, 6:28 PM

Fixed another bug in the implemantation to make it consistent with gnu assembler.

PTAL, Thanks

What did you fix? :)

-eric

dblaikie accepted this revision.Apr 28 2016, 3:01 PM
dblaikie edited edge metadata.
This revision is now accepted and ready to land.Apr 28 2016, 3:01 PM
danielcdh closed this revision.Apr 28 2016, 3:15 PM