Page MenuHomePhabricator

[LoongArch] Support lowering FrameIndex
ClosedPublic

Authored by wangleiat on Jun 23 2022, 5:14 AM.

Details

Summary

Ensure callee-saved registers are accessed relative to the stackpointer.

Depends on D128429

Diff Detail

Unit TestsFailed

TimeTest
60,030 msx64 debian > MLIR.Examples/standalone::test.toy
Script: -- : 'RUN: at line 1'; /usr/bin/cmake /var/lib/buildkite-agent/builds/llvm-project/mlir/examples/standalone -G "Ninja" -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/var/lib/buildkite-agent/builds/llvm-project/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld
60,070 msx64 debian > ThreadSanitizer-x86_64.ThreadSanitizer-x86_64::restore_stack.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=thread -Wall -m64 -msse4.2 -gline-tables-only -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -O1 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/restore_stack.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/restore_stack.cpp.tmp && not /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/restore_stack.cpp.tmp 2>&1 | FileCheck /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/restore_stack.cpp

Event Timeline

wangleiat created this revision.Jun 23 2022, 5:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2022, 5:14 AM
wangleiat requested review of this revision.Jun 23 2022, 5:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2022, 5:14 AM
wangleiat updated this revision to Diff 440105.Jun 26 2022, 8:45 PM

Update test to use opaque pointers

xen0n added a comment.Jul 2 2022, 4:57 PM

The other changes look good.

llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
68–69

This implementation is a simplified version of RISCV's getFrameIndexReference, but we're missing a stack ID check before this. I know we're not utilizing any non-default stack IDs so far but I don't know if such a check/assertion should be kept anyway.

wangleiat added inline comments.Jul 3 2022, 6:09 PM
llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
68–69

Riscv uses stackID to handle ScalableVector, which we currently don't support, at least not currently.

xen0n accepted this revision.Jul 4 2022, 9:53 PM

LGTM apart from the stack ID assertion bit; @SixWeining do you think an assertion should be present nevertheless even if we're only using one type of stack currently?

This revision is now accepted and ready to land.Jul 4 2022, 9:53 PM

LGTM apart from the stack ID assertion bit; @SixWeining do you think an assertion should be present nevertheless even if we're only using one type of stack currently?

I think either is fine. I think the reason RISCV does an assertion is that it sets non-default stack ID in its .cpp code.
Considering LoongArch currently only use default stack ID which is not set by target explicitly, maybe there is no strong reason to do an assertion.

SixWeining accepted this revision.Jul 6 2022, 12:41 AM
This revision was landed with ongoing or failed builds.Jul 6 2022, 12:42 AM
This revision was automatically updated to reflect the committed changes.