[PATCH] D88896: [DebugInfo][InstrRef][3/4] Produce DBG_INSTR_REFs for all* variable locations
Jeremy Morse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 1 09:35:49 PDT 2021
jmorse added inline comments.
================
Comment at: llvm/include/llvm/CodeGen/MachineFunction.h:510
+ /// value, or None if nothing could be recovered.
+ Optional<DebugInstrOperandPair> salvageCopySSA(MachineInstr &MI);
+
----------------
Orlando wrote:
> Does this need to return an `Optional`? As far as I can tell `salvageCopySSA` never returns `None`, and the only callsite doesn't check whether the returned result has a value or not.
/me scratches head. I guess I prototyped this with it returning an optional, then revised it to assert if any value wasn't available.
I'll update this to not return an optional.
================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:1039-1040
+ Offset = TRI.getSubRegIdxOffset(SubReg);
+ if (OldRegBits >= NewRegBits || Offset)
+ SubReg = 0;
+
----------------
Orlando wrote:
> I'm struggling to follow this part. When you say "only remember truncations or offsets" am I right in thinking that setting `SubReg = 0` causes some kind of mismatch that is picked up and handled by LiveDebugValues/InstrRefBasedImpl in D88894?
Probably because it's a cop-out -- if I compile this:
void ext(int b);
int foo(char a)
{
ext(a);
return 0;
}
I get `DBG_VALUE $edi` for `b` rather than `DBG_VALUE $al`, which has encouraged me to be fairly lax about accurate widening for parameters. Off the top of my head, I can't think of any reason to omit the subregister information, I'll run some experiments to see what effect it has.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88896/new/
https://reviews.llvm.org/D88896
More information about the llvm-commits
mailing list