[PATCH] D72067: [PowerPC] Delete PPCSubtarget::isDarwin and isDarwinABI
Sean Fertile via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 2 10:26:54 PST 2020
sfertile added a comment.
I //think// we should be safe removing the Darwin code in this patch, but Chris will know better.
The clang-format and clang-tidy comments are pretty cool, but whats expected when they show up on code that wasn't changed? We have a lot of code in the PowerPC directory which will trigger these. I've never bothered with cleaning that code up because I though keeping the history was more important than fixing the formatting. If these are going to be flooding every review though maybe its better we start to fix them.
================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:488
- if (!Subtarget->isPPC64() && !Subtarget->isDarwin() &&
- isPositionIndependent())
+ if (!Subtarget->isPPC64() && isPositionIndependent())
Kind = MCSymbolRefExpr::VK_PLT;
----------------
`if (Subtarget->is32BitELFABI() && isPositionIndependent())`
================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:1804
// function uses CR 2, 3, or 4.
- if (!isPPC64 && !IsDarwinABI &&
- (SavedRegs.test(PPC::CR2) ||
- SavedRegs.test(PPC::CR3) ||
- SavedRegs.test(PPC::CR4))) {
+ if (!isPPC64 && (SavedRegs.test(PPC::CR2) || SavedRegs.test(PPC::CR3) ||
+ SavedRegs.test(PPC::CR4))) {
----------------
If the comment is correct, the proper check would be `if (Subtarget.is32BitELFABI() && (...))`. I believe @cebowleratibm should be able to clarify what AIX's behavior should be.
================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:2178
unsigned Reg = CSI[i].getReg();
// Only Darwin actually uses the VRSAVE register, but it can still appear
// here if, for example, @llvm.eh.unwind.init() is used. If we're not on
----------------
Comment needs to be updated.
================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:2351
// Only Darwin actually uses the VRSAVE register, but it can still appear
// here if, for example, @llvm.eh.unwind.init() is used. If we're not on
----------------
Comment needs to be updated.
================
Comment at: llvm/lib/Target/PowerPC/PPCSubtarget.cpp:147
- if (isDarwin())
- HasLazyResolverStubs = true;
-
----------------
I believe the member variable `HasLazyResolverStubs` should be dead with the Darwin removal. Likewise for `PPCSubtarget::hasLazyResolverStub`.
================
Comment at: llvm/lib/Target/PowerPC/PPCSubtarget.h:297
- // DarwinABI has a 224-byte red zone. PPC32 SVR4ABI(Non-DarwinABI) has no
- // red zone and PPC64 SVR4ABI has a 288-byte red zone.
+ // PPC32 SVR4ABI(Non-DarwinABI) has no red zone and PPC64 SVR4ABI has a
+ // 288-byte red zone.
----------------
minor nit: drop the `(Non-DarwinABI)` from the comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72067/new/
https://reviews.llvm.org/D72067
More information about the llvm-commits
mailing list