[llvm] [StackFrameLayoutAnalysis] Use target-specific hook for SP offsets (PR #100386)

Paul Kirth via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 24 10:11:31 PDT 2024


ilovepi wrote:

> @ilovepi Thanks for reviewing. I was also looking at using the getFrameIndexReference() API directly - but we were keen to have all offsets from a single point of reference. Using the DWARF logic to get CFA relative offsets sounds promising, but maybe this way is simpler for now.
> 
> > Somehow the YAML tests are still passing, though, which I find surprising. Likely that means we don't have enough coverage. I can follow up on that, and also additional RISC-V tests w/ scalable vectors.
> 
> That doesn’t completely surprise me, if these are non AArch64 tests - this shouldn’t change the YAML output for non AArch64 targets. And should only add the “ScalableOffset” field to the YAML for AArch64 when the value is non-zero.
> 
> Are you able to point me to the tests you’re referencing? Thanks

https://github.com/llvm/llvm-project/blob/main/clang/test/Frontend/stack-layout-remark.c, but I can see why those aren't  affected, since it `REQUIRES: x86-registered-target`. It may not be too important. The point of that test is to make sure that the YAML output is functional. While your patch modifies the output for Offset, I don't think its impacted any functionallity. Still, I'll probably add a Aarch64 test too, so we have some coverage on Scalable Vectors.

https://github.com/llvm/llvm-project/pull/100386


More information about the llvm-commits mailing list