[PATCH] D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

In D90046#2437220 <https://reviews.llvm.org/D90046#2437220>, @jmorse wrote:

> Forgive my ignorance on scalable vectors; but is there definitely a scenario where the "Scalable" portion of the stack offset is nonzero, and if so could the test cover it please.

No worries! In the test-case in this patch the Scalable portion of the offset is actually non-zero:

  # CHECK-DAG: ST1W_IMM renamable $z1, killed renamable $p{{[0-9]+}}, $fp, -[[#%u,OFFSET:]]
                                                                              ^^^^^^^^^^^

For this test OFFSET is '1' (so it is stored at FP - 1 * (length of scalable vector)). The 'length of scalable vector' is handled by the addressing mode of ST1W. So for 128bit vectors, it would store to FP - 16bytes, for 256 it would store to FP - 32bytes, etc. The corresponding StackOffset returned by `TFI->getFrameIndexReference` is `{0 bytes, -16 scalable bytes}`, and because SVE vectors are always a multiple of 16 bytes, this translates to using `-1` for ST1W_IMM.

After the live-debugvalues pass, the debug-location for this value is now described by:

  CHECK-DAG: DBG_VALUE $fp, 0, !{{[0-9]+}}, !DIExpression(DW_OP_constu, [[#mul(OFFSET,8)]], DW_OP_bregx, 46, 0, DW_OP_mul, DW_OP_minus),

Which describes the location `FP - 8 * VG`, where VG is the number of 64-bit 'vector granules'. For an 128bit vector, VG is 2, for 256 it is 4, etc, resulting in the same addresses calculated by the ST1W instruction.

> Right now I believe that for aggregate types:
>
> - We can refer to portions of structs if they're split by SROA and promoted from there,
> - The same for arrays that are completely promoted,
> - All other aggregates are left on the stack, and we only describe their stack address.
>
> As a result, the stack-location tracking in LiveDebugValues is only geared towards tracking the spills / restores of scalars that the register allocator introduces, and sometimes entire vectors. I have a fear that if this patch is trying to make elements within scalable vectors describable to debug-info, it might be aiming to do something that isn't supported.

I don't think this is any different for scalable vectors. Specifically because (from the LangRef):

  Scalable vectors cannot be global variables or members of structs or arrays because their size is unknown at compile time.

So I would expect only entire vectors to be tracked. It is also worth noting that the current use for this is with source-level scalable vector types. For SVE the only scalable types are defined in `arm_sve.h` which can only be used in conjunction with the intrinsics defined in the Arm C/C++ Language Extensions for SVE.

> This probably stems from not understanding scalable vectors; would you have any source examples of when this patch becomes necessary, just so that I can work through them?

The following example hits the assert in `VarLocBasedLDV::extractSpillBaseRegAndOffset` that the StackOffset has no scalable component (which it now has, because the stack contains two SVE locals, at least the address of one of them must have a non-zero scalable offset).

  // After applying D90020, build this example with:
  //   clang -O1 -g -march=armv8-a+sve2 -S t.c -o -
  
  #include <arm_sve.h>
  
  svint32_t bar(svint32_t &);
  
  svint32_t foo(svint32_t v, int N) {
    svint32_t local_v1 = v;
    svint32_t local_v2 = v;
    if (N > 0) {
      local_v1 = bar(local_v1);
      local_v2 = bar(local_v2);
    }
    return svadd_z(svptrue_b32(), local_v1, local_v2);
  }

Hopefully that clarifies a few things, but don't hesitate to ask if you have more questions!


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

https://reviews.llvm.org/D90046