[PATCH] D58042: [LiveDebugValues] Emit parameter's entry value
Djordje Todorovic via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 20 06:59:02 PDT 2019
djtodoro marked 5 inline comments as done.
djtodoro added inline comments.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:455
+ const MachineInstr *CurrDebugInstr = &VarLocIDs[ID].MI;
+ auto DebugInstr = std::find_if(DebugEntryVals.begin(), DebugEntryVals.end(),
+ [&CurrDebugInstr](MachineInstr *MII) {
----------------
aprantl wrote:
> This is going into (low) n^2 territory here. Should we make DebugEntryVals a SmallPtrSet to avoid catastrophic behavior for functions with many arguments?
Mmm, we see.. But we will need another set for caching parameters debug variables, since we consider a parameters `DBG_VALUE` as a "new" if didn't encountered a debug variable from it.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:885
const VarLoc &DiffIt = VarLocIDs[ID];
- const MachineInstr *DMI = &DiffIt.MI;
+ const MachineInstr *DebugInstr = &DiffIt.MI;
MachineInstr *MI =
----------------
aprantl wrote:
> Can you land this as an NFC refactoring (no need for a separate phab review?)
Sure.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:950
+ SmallVector<MachineInstr *, 8> DebugEntryVals;
+ auto IsNewParameter = [&DebugEntryVals](const MachineInstr &MI) -> bool {
+ return !std::any_of(DebugEntryVals.begin(), DebugEntryVals.begin(),
----------------
aprantl wrote:
> What does "New" mean here?
It means that a parameter is not collected yet inside the `DebugEntryVals`. We should name it `IsParamEntryValueCollected` or something like that.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:966
+ if (MI.isDebugValue() && IsUnmodifiedFuncParam(MI) &&
+ IsRegOtherThanSPAndFP(MI.getOperand(0)) && IsNewParameter(MI) &&
+ !IsFragment(MI.getDebugExpression()))
----------------
aprantl wrote:
> We need a longer comment here that explains why each of these conditions are necessary and what is being checked for.
Sure. We will add that.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:967
+ IsRegOtherThanSPAndFP(MI.getOperand(0)) && IsNewParameter(MI) &&
+ !IsFragment(MI.getDebugExpression()))
+ DebugEntryVals.push_back(&MI);
----------------
aprantl wrote:
> Why can't fragments be entry values?
Fragments can be entry values as well, but don't support it in this initial set of patches.
We will leave a `TODO` for that here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58042/new/
https://reviews.llvm.org/D58042
More information about the llvm-commits
mailing list