[PPC] In PPCBoolRetToInt change the bool value to i64 if the target is ppc64

[Carrot]

Summary

In PPCBoolRetToInt bool value is changed to i32 type. On ppc64 it may introduce an extra zero extension for the return value. This patch changes the integer type to i64 to avoid the zero extension on ppc64.

This patch fixed PR32442.

[echristo]

Couple of small nits, otherwise LGTM.

-eric

lib/Target/PowerPC/PPCBoolRetToInt.cpp

91

Update?

197

Let's just always assume we have a TargetMachine and return false if not?

test/CodeGen/PowerPC/pr32442.ll

2

Throw the PR number in here as a comment?

[nemanjai]

lib/Target/PowerPC/PPCBoolRetToInt.cpp

93

Just a personal preference nit...
Maybe turn this into a conditional operator for the initialization (rather than the assignment in the if).

[Carrot]

test/CodeGen/PowerPC/pr32442.ll

2

It is contained in the filename.

[echristo]

test/CodeGen/PowerPC/pr32442.ll

2

So it is. Perhaps a more descriptive filename and a link to the PR :)

[echristo]

test/CodeGen/PowerPC/pr32442.ll

2

Any reason this can't go into the other file?

[Carrot]

test/CodeGen/PowerPC/pr32442.ll

2

Test case BoolRetToIntTest.ll runs BoolRetToInt pass only, and check the LLVM IR. The extra zero extension is not very obvious at this time, it is explicitly generated at the machine instruction phase.

[echristo]

test/CodeGen/PowerPC/pr32442.ll

2

Aha. That makes sense.

How about BoolRetToIntTest-2.ll then and leave the PR in the comment section?

I realize this seems like a crazy amount of care over file naming, but historically I've found that pr named tests aren't very helpful.

[echristo]

LGTM.

Thanks!

[Carrot]

The first commit triggered bug pr32608, and was reverted by r299153. Now the bug has been fixed by r304001, so I will try to commit it again. Unfortunately INITIALIZE_TM_PASS has been removed and TargetMachine access mechanism is changed, so I modified the patch to use new mechanism.

Please take another look.

[echristo]

One inline nit then LGTM.

-eric

lib/Target/PowerPC/PPCBoolRetToInt.cpp

197

If you get a PPCTargetMachine you shouldn't have to cast the subtarget.

lib/Target/PowerPC/PPC.h

   FunctionPass *createPPCVSXCopyPass();
   FunctionPass *createPPCVSXFMAMutatePass();
   FunctionPass *createPPCVSXSwapRemovalPass();
   FunctionPass *createPPCMIPeepholePass();
   FunctionPass *createPPCBranchSelectionPass();
   FunctionPass *createPPCQPXLoadSplatPass();
   FunctionPass *createPPCISelDag(PPCTargetMachine &TM);
   FunctionPass *createPPCTLSDynamicCallPass();
-  FunctionPass *createPPCBoolRetToIntPass();
+  FunctionPass *createPPCBoolRetToIntPass(PPCTargetMachine &TM);
   FunctionPass *createPPCExpandISELPass();
   void LowerPPCMachineInstrToMCInst(const MachineInstr *MI, MCInst &OutMI,
                                     AsmPrinter &AP, bool isDarwin);
 
   void initializePPCVSXFMAMutatePass(PassRegistry&);
   void initializePPCBoolRetToIntPass(PassRegistry&);
   void initializePPCExpandISELPass(PassRegistry &);
   extern char &PPCVSXFMAMutateID;

lib/Target/PowerPC/PPCBoolRetToInt.cpp

 // There's code in SimplifyCFG that code be used to turn control flow in data
 // flow using SelectInsts. Selects are slow on some architectures (P7/P8), so
 // this probably isn't good in general, but for the special case of i1, the
 // Selects could be further lowered to bit operations that are fast everywhere.
 //
 //===----------------------------------------------------------------------===//
 
 #include "PPC.h"
+#include "PPCTargetMachine.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Dominators.h"

       if (CurrUser && !isa<CallInst>(Curr))
         for (auto &Op : CurrUser->operands())
           if (Defs.insert(Op).second)
             WorkList.push_back(Op);
     }
     return Defs;
   }
 
   // Translate a i1 value to an equivalent i32 value:
-  static Value *translate(Value *V) {
-    Type *Int32Ty = Type::getInt32Ty(V->getContext());
+  Value *translate(Value *V) {
+    Type *IntTy = Type::getInt32Ty(V->getContext());
+    if (ST && ST->isPPC64())
+      IntTy = Type::getInt64Ty(V->getContext());
+
     if (auto *C = dyn_cast<Constant>(V))
-      return ConstantExpr::getZExt(C, Int32Ty);
+      return ConstantExpr::getZExt(C, IntTy);
     if (auto *P = dyn_cast<PHINode>(V)) {
       // Temporarily set the operands to 0. We'll fix this later in
       // runOnUse.
-      Value *Zero = Constant::getNullValue(Int32Ty);
+      Value *Zero = Constant::getNullValue(IntTy);
       PHINode *Q =
-        PHINode::Create(Int32Ty, P->getNumIncomingValues(), P->getName(), P);
+        PHINode::Create(IntTy, P->getNumIncomingValues(), P->getName(), P);
       for (unsigned i = 0; i < P->getNumOperands(); ++i)
         Q->addIncoming(Zero, P->getIncomingBlock(i));
       return Q;
     }
 
     auto *A = dyn_cast<Argument>(V);
     auto *I = dyn_cast<Instruction>(V);
     assert((A || I) && "Unknown value type");
 
     auto InstPt =
       A ? &*A->getParent()->getEntryBlock().begin() : I->getNextNode();
-    return new ZExtInst(V, Int32Ty, "", InstPt);
+    return new ZExtInst(V, IntTy, "", InstPt);
   }
 
   typedef SmallPtrSet<const PHINode *, 8> PHINodeSet;
 
   // A PHINode is Promotable if:
   // 1. Its type is i1 AND
   // 2. All of its uses are ReturnInt, CallInst, PHINode, or DbgInfoIntrinsic
   // AND

     return Promotable;
   }
 
   typedef DenseMap<Value *, Value *> B2IMap;
 
  public:
   static char ID;
 
-  PPCBoolRetToInt() : FunctionPass(ID) {
+  PPCBoolRetToInt() : FunctionPass(ID), TM(nullptr) {
+    initializePPCBoolRetToIntPass(*PassRegistry::getPassRegistry());
+  }
+
+  PPCBoolRetToInt(PPCTargetMachine &TM) : FunctionPass(ID), TM(&TM) {
     initializePPCBoolRetToIntPass(*PassRegistry::getPassRegistry());
   }
 
   bool runOnFunction(Function &F) override {
     if (skipFunction(F))
       return false;
 
+    ST = nullptr;
+    if (TM)
+      ST = TM->getSubtargetImpl(F);
+
     PHINodeSet PromotablePHINodes = getPromotablePHINodes(F);
     B2IMap Bool2IntMap;
     bool Changed = false;
     for (auto &BB : F) {
       for (auto &I : BB) {
         if (auto *R = dyn_cast<ReturnInst>(&I))
           if (F.getReturnType()->isIntegerTy(1))
             Changed |=
               runOnUse(R->getOperandUse(0), PromotablePHINodes, Bool2IntMap);
 
         if (auto *CI = dyn_cast<CallInst>(&I))
           for (auto &U : CI->operands())
             if (U->getType()->isIntegerTy(1))
               Changed |= runOnUse(U, PromotablePHINodes, Bool2IntMap);
       }
     }
 
     return Changed;
   }
 
-  static bool runOnUse(Use &U, const PHINodeSet &PromotablePHINodes,
+  bool runOnUse(Use &U, const PHINodeSet &PromotablePHINodes,
                        B2IMap &BoolToIntMap) {
     auto Defs = findAllDefs(U);
 
     // If the values are all Constants or Arguments, don't bother
     if (llvm::none_of(Defs, isa<Instruction, Value *>))
       return false;
 
     // Presently, we only know how to handle PHINode, Constant, Arguments and

 
     return true;
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addPreserved<DominatorTreeWrapperPass>();
     FunctionPass::getAnalysisUsage(AU);
   }
+
+private:
+  const PPCSubtarget *ST;
+  PPCTargetMachine *TM;
 };
 
 } // end anonymous namespace
 
 char PPCBoolRetToInt::ID = 0;
 INITIALIZE_PASS(PPCBoolRetToInt, "bool-ret-to-int",
                 "Convert i1 constants to i32 if they are returned",
                 false, false)
 
-FunctionPass *llvm::createPPCBoolRetToIntPass() { return new PPCBoolRetToInt(); }
+FunctionPass *llvm::createPPCBoolRetToIntPass(PPCTargetMachine &TM) {
+  return new PPCBoolRetToInt(TM);
+}

lib/Target/PowerPC/PPCTargetMachine.cpp

 } // end anonymous namespace
 
 TargetPassConfig *PPCTargetMachine::createPassConfig(PassManagerBase &PM) {
   return new PPCPassConfig(this, PM);
 }
 
 void PPCPassConfig::addIRPasses() {
   if (TM->getOptLevel() != CodeGenOpt::None)
-    addPass(createPPCBoolRetToIntPass());
+    addPass(createPPCBoolRetToIntPass(getPPCTargetMachine()));
   addPass(createAtomicExpandPass(&getPPCTargetMachine()));
 
   // For the BG/Q (or if explicitly requested), add explicit data prefetch
   // intrinsics.
   bool UsePrefetching = TM->getTargetTriple().getVendor() == Triple::BGQ &&
                         getOptLevel() != CodeGenOpt::None;
   if (EnablePrefetch.getNumOccurrences() > 0)
     UsePrefetching = EnablePrefetch;

test/CodeGen/PowerPC/pr32442.ll

+; RUN: llc -mtriple=powerpc64le-linux-gnu -mcpu=pwr8 < %s | FileCheck %s
+
+; CHECK-NOT: clrldi
+
+define zeroext i1 @foo(i32 signext %i, i32* %p) {
+entry:
+  %cmp = icmp eq i32 %i, 0
+  br i1 %cmp, label %return, label %if.end
+
+if.end:
+  store i32 %i, i32* %p, align 4
+  br label %return
+
+return:
+  %retval = phi i1 [ true, %if.end ], [ false, %entry ]
+  ret i1 %retval
+}
+