[llvm] Remove uses of Type::getPointerTo() (NFC)

[JOE1994]

Summary

Partial progress towards removing in-tree uses of getPointerTo(),
by employing the following options:

• Drop the call entirely if the sole purpose of it is to support a no-op bitcast (remove the no-op bitcast as well).

• Replace with PointerType::get()/PointerType::getUnqual()

This is a NFC cleanup effort.

[JOE1994]

• Apply clang-format

[JOE1994]

llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp

687

Given that this line is guarded by if (!PtrTy->isOpaque(),
I wasn't sure whether using PointerType::getUnqual(LLVMContext&) here is acceptable.

For now, I chose to play safe and just used PointerType::getUnqual(Type *) here.

[barannikov88]

I think getting rid of typed pointers should be an actual cleanup, not just a textual replacement. I outlined a few possible simplifications, but I didn't analyze deeply.

llvm/lib/CodeGen/AtomicExpandPass.cpp

728 ↗

(On Diff #540180)

This is the same as PtrTy.

llvm/lib/CodeGen/CodeGenPrepare.cpp

7852 ↗

(On Diff #540180)

llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp

687

isOpaque() always returns true, this whole block can be removed.

687–692

Isn't this a no-op? And the one above?

llvm/lib/Target/X86/X86TargetTransformInfo.cpp

5763–5764

clang-format does poorly when argument comments are involved.

llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

590 ↗

(On Diff #540180)

The condition is always true because the bitcast can never be matched.

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

193–195

Would that be correct?

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3385

This cast looks no-op.

llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp

38 ↗

(On Diff #540180)

[JOE1994]

Apply feedback.

Currently based on old commit from last July.
Needs rebasing.

[JOE1994]

llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp

687–692

Yes, CreatePointerCast on line 713 is a no-op, given that Tmp1 is a pointer with address space 0
and that the cast target type (on line 713) is also a pointer type with address space 0.

If Ptr is guaranteed to have address space 0, CreatePointerCast on line 710 would also be a no-op.

Given the comment on line 695 (added in f5d07a05bbd41f827ccfa1bed7bfdfbab2be85dc),
the original author likely meant to do bitcasts but just used pointer casts.

Removing both pointercasts doesn't cause any regressions in ninja check-llvm.

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

193–195

LIT test LLVM :: Transforms/PGOProfile/counter_promo_with_bias.ll fails with this change.

The suggested change should be updated to be Addr = Builder.Insert(AddrInst->clone());.

Though even after that,
in the suggested change Addr is IntToPtr (OrigBiastInst),
while in previous code Addr is IntToPtr (BiasInst->clone()) .

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3385

Thank you for noticing this.
Removing the pointercast doesn't cause any regressions in ninja check-llvm.

[JOE1994]

Rebased onto latest main

[JOE1994]

Would it be alright to merge this? After reading onto latest main, I think the associated changes are small and straightforward :)

[barannikov88]

LGTM

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

for (const SDValue &Op : Node->op_values()) {
     Entry.IsSExt = isSigned;
     Entry.IsZExt = !isSigned;
     Args.push_back(Entry);
   }
 
   // Also pass the return address of the remainder.
   SDValue FIPtr = DAG.CreateStackTemporary(RetVT);
   Entry.Node = FIPtr;
-  Entry.Ty = RetTy->getPointerTo();
+  Entry.Ty = PointerType::getUnqual(RetTy->getContext());
   Entry.IsSExt = isSigned;
   Entry.IsZExt = !isSigned;
   Args.push_back(Entry);
 
   SDValue Callee = DAG.getExternalSymbol(TLI.getLibcallName(LC),
                                          TLI.getPointerTy(DAG.getDataLayout()));
 
   SDLoc dl(Node);
SelectionDAGLegalize::ExpandSinCosLibCall(SDNode *Node,
   Entry.Ty = RetTy;
   Entry.IsSExt = false;
   Entry.IsZExt = false;
   Args.push_back(Entry);
 
   // Pass the return address of sin.
   SDValue SinPtr = DAG.CreateStackTemporary(RetVT);
   Entry.Node = SinPtr;
-  Entry.Ty = RetTy->getPointerTo();
+  Entry.Ty = PointerType::getUnqual(RetTy->getContext());
   Entry.IsSExt = false;
   Entry.IsZExt = false;
   Args.push_back(Entry);
 
   // Also pass the return address of the cos.
   SDValue CosPtr = DAG.CreateStackTemporary(RetVT);
   Entry.Node = CosPtr;
-  Entry.Ty = RetTy->getPointerTo();
+  Entry.Ty = PointerType::getUnqual(RetTy->getContext());
   Entry.IsSExt = false;
   Entry.IsZExt = false;
   Args.push_back(Entry);
 
   SDValue Callee = DAG.getExternalSymbol(TLI.getLibcallName(LC),
                                          TLI.getPointerTy(DAG.getDataLayout()));
 
   SDLoc dl(Node);

llvm/lib/IR/AutoUpgrade.cpp

if (IsX86 && (Name.startswith("sse2.pcmp") ||
       Rep = Builder.CreateCall(CRC32, {Trunc0, CI->getArgOperand(1)});
       Rep = Builder.CreateZExt(Rep, CI->getType(), "");
     } else if (IsX86 && (Name.startswith("avx.vbroadcast.s") ||
                          Name.startswith("avx512.vbroadcast.s"))) {
       // Replace broadcasts with a series of insertelements.
       auto *VecTy = cast<FixedVectorType>(CI->getType());
       Type *EltTy = VecTy->getElementType();
       unsigned EltNum = VecTy->getNumElements();
-      Value *Cast = Builder.CreateBitCast(CI->getArgOperand(0),
-                                          EltTy->getPointerTo());
-      Value *Load = Builder.CreateLoad(EltTy, Cast);
+      Value *Load = Builder.CreateLoad(EltTy, CI->getArgOperand(0));
       Type *I32Ty = Type::getInt32Ty(C);
       Rep = PoisonValue::get(VecTy);
       for (unsigned I = 0; I < EltNum; ++I)
         Rep = Builder.CreateInsertElement(Rep, Load,
                                           ConstantInt::get(I32Ty, I));
     } else if (IsX86 && (Name.startswith("sse41.pmovsx") ||
                          Name.startswith("sse41.pmovzx") ||
                          Name.startswith("avx2.pmovsx") ||

llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp

auto AlignVectors::getPassThrough(Value *Val) const -> Value * {
   }
   return UndefValue::get(getPayload(Val)->getType());
 }
 
 auto AlignVectors::createAdjustedPointer(IRBuilderBase &Builder, Value *Ptr,
                                          Type *ValTy, int Adjust,
                                          const InstMap &CloneMap) const
     -> Value * {
   if (auto *I = dyn_cast<Instruction>(Ptr))
     if (Instruction *New = CloneMap.lookup(I))
       Ptr = New;
   return Builder.CreateGEP(Type::getInt8Ty(HVC.F.getContext()), Ptr,
                            HVC.getConstInt(Adjust), "gep");
 }
 
 auto AlignVectors::createAlignedPointer(IRBuilderBase &Builder, Value *Ptr,
                                         Type *ValTy, int Alignment,
                                         const InstMap &CloneMap) const
     -> Value * {
   auto remap = [&](Value *V) -> Value * {
     if (auto *I = dyn_cast<Instruction>(V)) {
       for (auto [Old, New] : CloneMap)
         I->replaceUsesOfWith(Old, New);
       return I;
     }
     return V;
   };
   Value *AsInt = Builder.CreatePtrToInt(Ptr, HVC.getIntTy(), "pti");
   Value *Mask = HVC.getConstInt(-Alignment);
   Value *And = Builder.CreateAnd(remap(AsInt), Mask, "and");
-  return Builder.CreateIntToPtr(And, ValTy->getPointerTo(), "itp");
+  return Builder.CreateIntToPtr(
+      And, PointerType::getUnqual(ValTy->getContext()), "itp");
 }
 
 auto AlignVectors::createLoad(IRBuilderBase &Builder, Type *ValTy, Value *Ptr,
                               Value *Predicate, int Alignment, Value *Mask,
                               Value *PassThru,
                               ArrayRef<Value *> MDSources) const -> Value * {
   bool HvxHasPredLoad = HVC.HST.useHVXV62Ops();
   // Predicate is nullptr if not creating predicated load

llvm/lib/Target/X86/X86TargetTransformInfo.cpp

if (VariableMask) {
     InstructionCost ScalarCompareCost = getCmpSelInstrCost(
         Instruction::ICmp, Type::getInt1Ty(SrcVTy->getContext()), nullptr,
         CmpInst::BAD_ICMP_PREDICATE, CostKind);
     InstructionCost BranchCost = getCFInstrCost(Instruction::Br, CostKind);
     MaskUnpackCost += VF * (BranchCost + ScalarCompareCost);
   }
 
   InstructionCost AddressUnpackCost = getScalarizationOverhead(
-      FixedVectorType::get(ScalarTy->getPointerTo(), VF), DemandedElts,
-      /*Insert=*/false, /*Extract=*/true, CostKind);
+      FixedVectorType::get(PointerType::getUnqual(ScalarTy->getContext()), VF),
+      DemandedElts, /*Insert=*/false, /*Extract=*/true, CostKind);
 
   // The cost of the scalar loads/stores.
   InstructionCost MemoryOpCost =
       VF * getMemoryOpCost(Opcode, ScalarTy, MaybeAlign(Alignment),
                            AddressSpace, CostKind);
 
   // The cost of forming the vector from loaded scalars/
   // scalarizing the vector to perform scalar stores.

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

for (unsigned i = 0, e = ExitBlocks.size(); i != e; ++i) {
         // the store instruction is computed with two instructions in
         // InstrProfiling::getCounterAddress(). We need to copy those
         // instructions to this block to compute Addr correctly.
         // %BiasAdd = add i64 ptrtoint <__profc_>, <__llvm_profile_counter_bias>
         // %Addr = inttoptr i64 %BiasAdd to i64*
         auto *OrigBiasInst = dyn_cast<BinaryOperator>(AddrInst->getOperand(0));
         assert(OrigBiasInst->getOpcode() == Instruction::BinaryOps::Add);
         Value *BiasInst = Builder.Insert(OrigBiasInst->clone());
-        Addr = Builder.CreateIntToPtr(BiasInst, Ty->getPointerTo());
+        Addr = Builder.CreateIntToPtr(BiasInst,
+                                      PointerType::getUnqual(Ty->getContext()));
       }
       if (AtomicCounterUpdatePromoted)
         // automic update currently can only be promoted across the current
         // loop, not the whole loop nest.
         Builder.CreateAtomicRMW(AtomicRMWInst::Add, Addr, LiveInValue,
                                 MaybeAlign(),
                                 AtomicOrdering::SequentiallyConsistent);
       else {
         LoadInst *OldVal = Builder.CreateLoad(Ty, Addr, "pgocount.promoted");

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

void handleVectorReduceAndIntrinsic(IntrinsicInst &I) {
     setShadow(&I, S);
     setOrigin(&I, getOrigin(&I, 0));
   }
 
   void handleStmxcsr(IntrinsicInst &I) {
     IRBuilder<> IRB(&I);
     Value *Addr = I.getArgOperand(0);
     Type *Ty = IRB.getInt32Ty();
-    Type *PtrTy = IRB.getPtrTy();
     Value *ShadowPtr =
         getShadowOriginPtr(Addr, IRB, Ty, Align(1), /*isStore*/ true).first;
 
-    IRB.CreateStore(getCleanShadow(Ty),
-                    IRB.CreatePointerCast(ShadowPtr, PtrTy));
+    IRB.CreateStore(getCleanShadow(Ty), ShadowPtr);
 
     if (ClCheckAccessAddress)
       insertShadowCheck(Addr, &I);
   }
 
   void handleLdmxcsr(IntrinsicInst &I) {
     if (!InsertChecks)
       return;

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

const auto CreateVecPtr = [&](unsigned Part, Value *Ptr) -> Value * {
     // Calculate the pointer for the specific unroll-part.
     Value *PartPtr = nullptr;
 
     // Use i32 for the gep index type when the value is constant,
     // or query DataLayout for a more suitable index type otherwise.
     const DataLayout &DL =
         Builder.GetInsertBlock()->getModule()->getDataLayout();
     Type *IndexTy = State.VF.isScalable() && (isReverse() || Part > 0)
-                        ? DL.getIndexType(ScalarDataTy->getPointerTo())
+                        ? DL.getIndexType(PointerType::getUnqual(
+                              ScalarDataTy->getContext()))
                         : Builder.getInt32Ty();
     bool InBounds = false;
     if (auto *gep = dyn_cast<GetElementPtrInst>(Ptr->stripPointerCasts()))
       InBounds = gep->isInBounds();
     if (isReverse()) {
       // If the address is consecutive but reversed, then the
       // wide store needs to start at the last vector element.
       // RunTimeVF =  VScale * VF.getKnownMinValue()