Page MenuHomePhabricator

[NVPTX] emit .file directives for files referenced by subprograms.
ClosedPublic

Authored by tra on Feb 10 2016, 11:19 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

tra updated this revision to Diff 47494.Feb 10 2016, 11:19 AM
tra retitled this revision from to [NVPTX] emit .file directives for files referenced by subprograms..
tra updated this object.
tra added reviewers: jholewinski, jlebar.
tra added a subscriber: llvm-commits.

Needs a test case, presumably?

jlebar accepted this revision.Feb 10 2016, 11:23 AM
jlebar edited edge metadata.

Oh cool, there are no tests for this asm printer at all?

This revision is now accepted and ready to land.Feb 10 2016, 11:23 AM
jholewinski edited edge metadata.Feb 10 2016, 11:32 AM

We do not have any tests for debug support, since it was never properly implemented. We have plenty of tests for other parts of the NVPTX AsmPrinter. Or are you referring to something else?

As for the review, the change looks good, but a test should be included.

jlebar requested changes to this revision.Feb 10 2016, 11:35 AM
jlebar edited edge metadata.

We do not have any tests for debug support, since it was never properly implemented. We have plenty of tests for other parts of the NVPTX AsmPrinter. Or are you referring to something else?

Oh, no, I just fail at grep. For some reason I was looking for non-.ll files.

This revision now requires changes to proceed.Feb 10 2016, 11:35 AM
tra updated this revision to Diff 47507.Feb 10 2016, 12:49 PM
tra edited edge metadata.

Added test case.

dblaikie accepted this revision.Feb 10 2016, 2:02 PM
dblaikie added a reviewer: dblaikie.

Looks good - but please simplify the test a bit further and include the basic C code that produced the IR (I realize the IR is fairly simple, but it's handy to have what the code is modelling, the metadata format we have currently is still fairly verbose/hard to read)

The test could be simplier if it were C rather than C++ (no name mangling) and just void functions taking no parameters, I would imagine.

void foo() {
}
#line 1 other.c
void bar() {
}

Something like that, I would imagine.

Separately, this code seems strange - why does NVPTXAsmPrinter::recordAndEmitFilenames exist? It seems insufficient (what about file names on types? (DW_AT_decl_file on a user-defined type) why doesn't code like this exist for any other target? etc)

tra added a comment.Feb 10 2016, 2:20 PM

I'll simplify the test case.

Separately, this code seems strange - why does NVPTXAsmPrinter::recordAndEmitFilenames exist? It seems insufficient (what about file names on types? (DW_AT_decl_file on a user-defined type) why doesn't code like this exist for any other target? etc)

At the moment NVPTX only supports emitting line info and never produces any other debugging directives or data. I guess recordAndEmitFilenames() was the bare minimum needed to get line info working.

tra updated this revision to Diff 47531.Feb 10 2016, 2:36 PM
tra edited edge metadata.

Simplified test case and added original source used to create bitcode.

This revision was automatically updated to reflect the committed changes.