[PATCH] D85850: [PowerPC][AIX] Fix frame-pointer and base-pointer save/restore offset.
Sean Fertile via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 24 10:14:35 PDT 2020
sfertile added inline comments.
================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:1547
// Get the ABI.
bool isSVR4ABI = Subtarget.isSVR4ABI();
----------------
ZarkoCA wrote:
> I'm pretty sure this variable is no longer needed.
You are right, good catch.
================
Comment at: llvm/test/CodeGen/PowerPC/aix-base-pointer.ll:13
-define void @caller() {
%AlignedBuffer = alloca [32 x i32], align 32
----------------
ZarkoCA wrote:
> Just a question: is this test maybe good to have as it is to show behaviour for a void return?
>
I could have both a void return and a float return versions in the test, but I'm not sure its really worth the bother.
================
Comment at: llvm/test/CodeGen/PowerPC/aix-framepointer-save-restore.ll:2
+; RUN: llc -verify-machineinstrs < %s -mcpu=pwr4 -mattr=-altivec \
+; RUN: -mtriple=powerpc-ibm-aix-xcoff -frame-pointer=all | FileCheck %s \
+; RUN: -check-prefix=AIX32
----------------
Xiangling_L wrote:
> Since you have a variable `n` here, I believe we've already had some testcases making sure frame pointer will definitely be used in this scenario, you can remove `-frame-pointer=all`. But I am okay if you want to keep it just in case. And if so, should we also add on for below 64bit case?
Good point. I removed it from the 64 bit run step but missed this one.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85850/new/
https://reviews.llvm.org/D85850
More information about the llvm-commits
mailing list