Page MenuHomePhabricator

[mir] Serialize DILocation inline when not possible to use a metadata reference
ClosedPublic

Authored by dsanders on Dec 3 2018, 5:20 PM.

Details

Summary

Sometimes MIR-level passes create DILocations that were not present in the
LLVM-IR. For example, it may merge two DILocations together to produce a
DILocation that points to line 0.

Previously, the address of these DILocations were printed which prevented the
MIR from being read back into LLVM. With this patch, DILocations will use
metadata references where possible and fall back on serializing them inline like so:

MOV32mr %stack.0.x.addr, 1, _, 0, _, %0, debug-location !DILocation(line: 1, scope: !15)

Diff Detail

Repository
rL LLVM

Event Timeline

dsanders created this revision.Dec 3 2018, 5:20 PM
aprantl added inline comments.Dec 3 2018, 5:27 PM
lib/CodeGen/MIRParser/MIParser.cpp
1706 ↗(On Diff #176520)

I assume because of all the MIToken references there is no reasonable way that this code could be shared with LLParser?

dsanders marked 2 inline comments as done.Dec 3 2018, 6:17 PM
dsanders added inline comments.
lib/CodeGen/MIRParser/MIParser.cpp
1706 ↗(On Diff #176520)

I think a few small fragments are potentially sharable but the cost of doing so is going to outweigh the benefit. We'd have to find a way to keep the MIR lexer/parser in sync with LLParser's and be careful to avoid everything LLParser does with Instructions and other LLVM-IR level structures. It's probably best to keep the LLVM-IR and MIR parsers separate.

1788 ↗(On Diff #176520)

Should the line field be required? It's optional in LLParser but the MIR printer always writes it (even for line 0)

probinson added inline comments.
lib/CodeGen/MIRParser/MIParser.cpp
1706 ↗(On Diff #176520)

Sadly true. Bitcode Reader has metadata handling split out, but Bitcode Writer and the IR reader/writer all have IR and metadata handling mashed together. They'd need to be split apart first.

aprantl accepted this revision.Dec 6 2018, 2:52 PM
This revision is now accepted and ready to land.Dec 6 2018, 2:52 PM
This revision was automatically updated to reflect the committed changes.