Page MenuHomePhabricator

[DebugInfoMetadata] Refactor DIExpression::prepend constants (NFC)
ClosedPublic

Authored by djtodoro on May 15 2019, 6:36 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

djtodoro created this revision.May 15 2019, 6:36 AM

Thanks a lot, this is going to be a far less error-prone API!

include/llvm/IR/DebugInfoMetadata.h
2481 ↗(On Diff #199592)

I would either use unsigned or uint8_t here. uint16_t seems arbitrary.

2481 ↗(On Diff #199592)

DIExpression::DIExpressionFlags is probably overkill. How about DIExpression::PrependOps?

2482 ↗(On Diff #199592)

For readability: How about adding ApplyOffset = 0?

lib/CodeGen/LiveDebugValues.cpp
467 ↗(On Diff #199592)

Then this becomes DIExpression::ApplyOffset

lib/CodeGen/PrologEpilogInserter.cpp
1191 ↗(On Diff #199592)
bool WithStackValue = true;
DIExpr = DIExpression::prependOpcodes(DIExpr, Ops, WithStackValue);
lib/CodeGen/SafeStack.cpp
578 ↗(On Diff #199592)

same here

lib/Target/X86/X86OptimizeLEAs.cpp
574 ↗(On Diff #199592)

Expr = DIExpression::prepend(Expr, DIExpression::StackValue, AddrDispShift)

lib/Transforms/Instrumentation/AddressSanitizer.cpp
3112 ↗(On Diff #199592)

again, a bool NoDeref = false helper may improve readability.

lib/Transforms/Utils/InlineFunction.cpp
1820 ↗(On Diff #199592)

ditto.

djtodoro marked 9 inline comments as done.May 16 2019, 2:35 AM

@aprantl Thanks for your comments!

include/llvm/IR/DebugInfoMetadata.h
2482 ↗(On Diff #199592)

That sounds good to me.

djtodoro updated this revision to Diff 199772.May 16 2019, 2:37 AM

-Address suggestions
-Refactor the code a bit

aprantl accepted this revision.May 16 2019, 3:08 PM

Thanks! One bonus cleanup inline :-)

lib/Transforms/Utils/Local.cpp
1554 ↗(On Diff #199772)

We should probably just pass DIExprFlags, Offset instead of the two bools here.

This revision is now accepted and ready to land.May 16 2019, 3:08 PM

@aprantl

Thanks! One bonus cleanup inline :-)

Sure. This is done. Thanks!

djtodoro updated this revision to Diff 200004.May 17 2019, 2:39 AM

-Bounus refactor of replaceDbgDeclare()

aprantl accepted this revision.May 17 2019, 9:27 AM

LGTM.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2019, 3:33 AM