[PATCH] D79978: Call Frame Information (CFI) Handling for Basic Block Sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D79978#2146362 <https://reviews.llvm.org/D79978#2146362>, @dblaikie wrote:

> Whereas @maskray's suggestion of using explicit register use/clobber does seem a bit more explicit/intentional, rather than having to coax a certain representation out of the compiler backend, I guess something like:
>
>   void f3(bool b) { 
>     if (b) // adds a basic block
>        // clobber some example callee-save registers to force them to be callee-saved and to be described by cfi_offset directives
>       asm("nop" : : : "r12","r13","r14","r15");
>   }
>
>
> (at least I'm not sure much else is required - @MaskRay - was the second inline asm needed? or the return values/parameters? Also perhaps just testing one register would be sufficient? (less ordering issues, chance for unrelated backend changes to disturb this test))


The second inline asm in the example (in a previous comment of mine; pasted below for your convenience) is not needed. When I wrote it, I was thinking whether we can exercise more code in CFIInstrInserter, i.e. not just that (all non-entry basic blocks can inherit CFI from the entry), but also that: if a non-entry basic block has updated its CFI information, subsequent basic blocks can pick up that delta part (relative to the entry basic block)

  int f3(int i, int j, int k) {
    if (i == 0) { // adds a basic block
      asm("nop" : : : "rdi","rsi","rdx","rbp","r12","r13","r14","r15","memory"); // there is a .cfi_offset for each of rbp,r12,r13,r14,r15
      return j;
    }
    if (j == 0) { // adds a basic block
      // Ideally there is some code construct here which can reliably alter CFI, so that we can test that CFIInstrInserter can handle the delta part.
      // Unfortunately this cannot be inline asm `subq 100, %rsp` as that does not generate CFI_INSTRUCTION which can be tracked by CFIInstrInserter
      asm("xchg %%ax,%%ax" : : : "rdi","rsi","rdx","rbp","r14","r15","memory");  // r12 and r13 are not clobbered but the current implementation adds .cfi_offset for both r12 and r13
      return k;
    }
    return i;
  }

If we can find such a code construct, it'd give me more confidence that we are updating CFIInstrInserter correctly & it'd be more difficult to break the basic block sections code.

And yes that I used registers because I hope the order of these CFI_INSTRUCTION registers is relatively stable so `CHECK-NEXT: CFI_INSTRUCTION .....` can be a stronger test. As to offsets, I don't know codegen enough to confidently say that it is stable, but I hope simple code like this (with very specific spill slots) will not cause offsets to be updated abruptly.


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

https://reviews.llvm.org/D79978