Page MenuHomePhabricator

[RISCV] Avoid scalar outgoing arguments overwriting vector frame objects.
ClosedPublic

Authored by HsiangKai on Jun 3 2021, 8:48 AM.

Details

Summary

When using FP to access stack objects, the scalable stack objects will
be put at the lower end of the frame. It looks like

|-------------------|  <-- FP
| callee-saved regs |
|-------------------|
| scalar local vars |
|-------------------|
| RVV local vars    |
|-------------------|  <-- SP

If there are scalar arguments that need to pass through memory and there
are vector objects on the stack using FP to access. The outgoing scalar
arguments will overwrite the vector objects. It looks like

|-------------------|  <-- FP
| callee-saved regs |
|-------------------|
| scalar local vars |
|-------------------|         |-------------------|
| RVV local vars    |         | outgoing args     | <- outgoing arguments
|-------------------|  <-- SP |-------------------|    overwrite from here.

In this patch, we reserve the stack for the outgoing arguments before
function calls if using FP to access and there are scalable vector frame
objects. It looks like

|-------------------|  <-- FP
| callee-saved regs |
|-------------------|
| scalar local vars |
|-------------------|
| RVV local vars    |
|-------------------|
| outgoing args     |
|-------------------|  <-- SP

Diff Detail

Unit TestsFailed

TimeTest
360 msx64 debian > XRay-x86_64-linux.TestCases/Posix::basic-filtering.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fxray-instrument -m64 -std=c++11 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/xray/TestCases/Posix/basic-filtering.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/xray/X86_64LinuxConfig/TestCases/Posix/Output/basic-filtering.cpp.tmp -g

Event Timeline

HsiangKai created this revision.Jun 3 2021, 8:48 AM
HsiangKai requested review of this revision.Jun 3 2021, 8:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2021, 8:48 AM
craig.topper retitled this revision from [RISCV] Avoid scalar outgoing argumetns overwrite vector frame objects. to [RISCV] Avoid scalar outgoing arguments overwrite vector frame objects..Jun 3 2021, 9:00 AM

Thanks for the detailed explanation.

I think the title can be reworded as "overwriting" where it says "overwrite".

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
924

Update comment here, perhaps?

llvm/test/CodeGen/RISCV/rvv/rvv-out-arguments.ll
88

Just so I understand, this is presumably the bug here, right? We're storing a vector register to a0 which is s0 - (vlenb<<3) - 128 which is (in a roundabout way) sp? And we store the arguments to the same spots 0(sp) and 8(sp) on lines 116 and 115?

HsiangKai added inline comments.Jun 9 2021, 6:34 PM
llvm/test/CodeGen/RISCV/rvv/rvv-out-arguments.ll
88

Yeah, that is the problem. Out going arguments overwrite the vector value here. When users call the function, lots_args, second time, the vector value will be loaded from stack under -O0. Users will get the wrong value in the second call.

HsiangKai retitled this revision from [RISCV] Avoid scalar outgoing arguments overwrite vector frame objects. to [RISCV] Avoid scalar outgoing arguments overwriting vector frame objects..Jun 9 2021, 8:05 PM
HsiangKai updated this revision to Diff 351050.Jun 9 2021, 8:09 PM

Update comments.

frasercrmck accepted this revision.Jun 10 2021, 1:57 AM

LGTM other than my last question. Thanks!

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
925

I was expecting this to say "frame pointer" given my understanding and the code. It's very possible I'm mistaken, though.

This revision is now accepted and ready to land.Jun 10 2021, 1:57 AM
HsiangKai added inline comments.Jun 10 2021, 8:26 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
925

Yeah, it should be frame pointer. Thanks.

This revision was landed with ongoing or failed builds.Jun 10 2021, 9:27 PM
This revision was automatically updated to reflect the committed changes.