[PATCH] D72067: [PowerPC] Delete PPCSubtarget::isDarwin and isDarwinABI

[Sean Fertile via Phabricator via llvm-commits]

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