[PATCH] D78107: [CSInfo][MIPS] Call delay slot support in DwarfDebug

[Nikola Tesic via Phabricator via llvm-commits]

ntesic added a comment.

The main purpose of the "delay slot guard" is to ensure that call_site DIE won't be created for call with delay slot, because it could have incorrect "return PC" value (DW_AT_low_pc, DW_AT_call_return_pc).
This will happen if the call and its delay slot instruction are not in the expected order.

Result of the wrong instruction order in call-delay slot bundle is incorrect label insertion in DwarfDebug::beginInstruction().
Label will be inserted immediately after call instruction, but it should be inserted after delay slot instruction.
Please, notice label "$tmp2" after call instruction "jal" in the following assembler output.

  jal	f1
  $tmp2:
  	.loc	1 6 11                  # m.c:6:11
  	sw	$4, %lo(global)($1)

During creation of call_site DIEs in DwarfDebug::constructCallSiteEntryDIEs(), the incorrectly placed label is used and wrong value of "returnPC" interpreted.
So, the call return address points to the delay slot instruction and it should point to the instruction after.
Please, find value of "DW_AT_low_pc" which points to delay slot instruction "sw $4 ..." in the following dwarfdump and objdump outputs.

  0x00000067:     DW_TAG_GNU_call_site
                    DW_AT_abstract_origin	(0x00000079 "f1")
                    DW_AT_low_pc	(0x0000000000000010)



  c: 0c 00 00 00                  	jal	0 <foo>
        10: ac 24 00 00                  	sw	$4, 0($1)
        14: 24 02 00 7b                  	addiu	$2, $zero, 123 <foo+0x7b>

This problem for MIPS architecture was caused in LiveDebugValues pass, by insertion of DBG_VALUE after the call instruction, instead of insertion after whole bundle.
Please, find the following output after livedebugvalues pass, clearly incorrect.

  JAL @f1 ...{
  }
  DBG_VALUE $a0
  SW renamable $a0 

Fortunately, the livedebugvalues problem was fixed by https://reviews.llvm.org/D66888.

> Do you know what would happen if you didn't have this guard, and enabled call site information for another target with delay slots?

So, if the "call-delay slot" instruction order is not as expected, it will cause wrong "return PC" value.

> If it is something that is relatively easy to catch, or perhaps not very serious, I think I'd prefer to have this code target-agnostic (i.e. just MI.hasDelaySlot), and just leave it up to the developer enabling call site information for the target to make sure that there are no delay slot quirks that need to be considered.

We can use assertions in "delaySlotSupported" lambda, in order to ensure expected order of instructions (following example), instead of the target check.

  call_instruction {
     delay_slot_instruction
    }

In beginInstruction() guard we should expect that delay slot instruction is immediately after call instruction, before label insertion.
In constructCallSiteEntryDIEs() guard we should expect that both the call instruction and its successor are followed by the same label.

@dstenb @vsk @djtodoro Thanks for your comments and, please,
let me know what do you think about this approach.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78107/new/

https://reviews.llvm.org/D78107