[llvm] r185966 - Reapply an improved version of r180816/180817.

[Eric Christopher]

On Tue, Jul 9, 2013 at 2:53 PM, Adrian Prantl <aprantl at apple.com> wrote:
> Hey Eric,
>
> thanks for the comments!
>
>
> On Jul 9, 2013, at 2:15 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>>> bool LDVImpl::handleDebugValue(MachineInstr *MI, SlotIndex Idx) {
>>>   // DBG_VALUE loc, offset, variable
>>>   if (MI->getNumOperands() != 3 ||
>>> -      !MI->getOperand(1).isImm() || !MI->getOperand(2).isMetadata()) {
>>> +      !(MI->getOperand(1).isReg() || MI->getOperand(1).isImm()) ||
>>> +      !MI->getOperand(2).isMetadata()) {
>>
>> Mind commenting this a bit more on what we aren't handling?
>
> I’m not sure if I can come up with a comprehensive list of cases that are not handled. One case that I can think of would be a FrameIndex DBG_VALUE, but I’m not even sure whether they can even be introduced before LDV is run.
>

Yeah, I'm not positive either. It just seemed odd like it's checking
for the validity of the instruction at all instead of some case that
it's not handling.

>
>>> -  FuncInfo.ArgDbgValues.push_back(
>>> +    FuncInfo.ArgDbgValues.push_back(BuildMI(MF, getCurDebugLoc(),
>>> +                                            TII->get(TargetOpcode::DBG_VALUE),
>>> +                                            /* IsIndirect */ Offset != 0,
>>> +                                            Op->getReg(), Offset, Variable));
>>>
>>
>> Curious about the "Offset != 0" since we could have a 0 offset in the
>> dbg.value intrinsic coming in yes?
>
> I agree! I’m gradually pushing the problem up layer by layer. Currently this is just a more explicit notation of the status quo. I will add in a comment.
>

Thanks :)

-eric