[PATCH] D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component.
Sander de Smalen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 7 09:50:22 PST 2020
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
More information about the llvm-commits
mailing list