On Thu, Feb 8, 2018 at 7:56 AM, Alexandre Oliva <aol...@redhat.com> wrote: > On Feb 7, 2018, Jason Merrill <ja...@redhat.com> wrote: > >> OK, that makes sense. But I'm still uncomfortable with choosing an >> existing opcode for that purpose, which previously would have been >> chosen just for reasons of encoding complexity and size. > > Well, there's a good reason we didn't used to output this opcode: it's > nearly always the case that you're better off using a special opcode or > DW_LNS_advance_pc, that encodes the offset as uleb128 instead of a fixed > size. The only exceptions I can think of are offsets that have the most > significant bits set in the representable range for > DW_LNS_fixed_advance_pc (the uleb128 representation for > DW_LNS_advance_pc would end up taking an extra byte if insns don't get > more than byte alignment), and VLIW machines, in which the > DW_LNS_advance_pc operand needs to be multiplied by the ops-per-insns > (but also divided by the min-insn-length). So, unless you're creating a > gap of 16KiB to 64KiB in the middle of a function on an ISA such as > x86*, that has insns as small as 1 byte, you'll only use > DW_LNS_fixed_advance_pc when the assembler can't encode uleb128 offsets, > as stated in the DWARF specs.
Which is often true of non-gas assemblers, isn't it? > Well, now there's one more case for using > it, and it's a rare one as well. I didn't think it made sense to add > yet another opcode with a fixed-size operand for that. So, if I've got this right: The most conservative approach to updating the address is DW_LNE_set_address, but we definitely want that to reset the view because it's used for e.g. starting a new function. And if it resets the view, we need to be careful not to use it more than once for the same address. Special opcodes are only generated by the assembler from .loc directives, so we use symbolic views and let the assembler decide whether or not they reset the view. If we don't have that assembler support, and don't want to use DW_LNE_set_address, that leaves DW_LNS_advance_pc and DW_LNS_fixed_advance_pc. The former could be used by the assembler in cases when special opcodes can't express the necessary change compactly, whereas the latter won't be, so it's a better choice for this situation. And since we don't know whether the increment will be zero, we don't want it to reset the view. OK, that makes sense. Though I expect this will come up again when the DWARF committee looks at the proposal. > But then again, even it was an opcode used more often, it wouldn't be a > significant problem to assign it the special behavior of not resetting > the view counter. Views don't have to be reset for the whole thing to > work, we just need some means for the compiler (who doesn't know all > offsets) and debug info consumers (who do) to keep view numbers in sync. > A single opcode for the compiler to signal to the consumer that it > wasn't sure a smallish offset would be nonzero is enough, and > DW_LNS_fixed_advance_pc provides us with just what we need, without any > complications of having to compute a special opcode, or compute a > compiler-unknown offset times min-insn-length and have that encoded in > uleb128. > >> Thanks, it would be good to have this overview in a comment somewhere. > > You meant just these two paragraphs (like below), or the whole thing? I meant just the two paragraphs, thanks. Jason