Details
- Reviewers
aprantl dblaikie - Commits
- rGe101b07a1d7a: [Debug info] Transfer DI to fragment expressions for split integer values.
rG622fedc00150: [Debug info] Transfer DI to fragment expressions for split integer values.
rL311181: [Debug info] Transfer DI to fragment expressions for split integer values.
rL311102: [Debug info] Transfer DI to fragment expressions for split integer values.
Diff Detail
- Repository
- rL LLVM
Event Timeline
Thanks, I have a couple of smaller nitpicks, but this generally seems good to me.
llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp | ||
---|---|---|
829 ↗ | (On Diff #111404) | This comment basically just repeats the name of the function. I would either document what the function does, i.e. generate fragment expressions for the split up value, or remove it entirely. |
837 ↗ | (On Diff #111404) | llvm coding style prefers early exit over nesting, i.e: if (Dbg->getKind() !=SDNode) break; |
838 ↗ | (On Diff #111404) | perhaps change the return type in SDDbgValue instead? |
842 ↗ | (On Diff #111404) | why is the &* necessary? |
llvm/test/DebugInfo/MSP430/sdagsplit-1.ll | ||
20 ↗ | (On Diff #111404) | Hard-coding the metadata node numbers is not a good idea. Since you aren't checking their contents, why check for them at all? |
41 ↗ | (On Diff #111404) | can you remove all non-essential attributes (at least everything in quotes)? |
llvm/test/DebugInfo/X86/sdagsplit-1.ll | ||
18 ↗ | (On Diff #111404) | Probably better to convert this to checking the MIR output, too. |
41 ↗ | (On Diff #111404) | ditto |
Thanks!
llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp | ||
---|---|---|
853 ↗ | (On Diff #111436) | No need to do this now, but we might want to think about adding this functionality either to DIBuilder or IR/DebugInfo.cpp |