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

Youngsuk Kim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 21 03:07:28 PDT 2023


JOE1994 added inline comments.


================
Comment at: llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp:713-714
                                   remap(Tmp0), HVC.getConstInt(Adjust), "gep");
-  return Builder.CreatePointerCast(remap(Tmp1), ValTy->getPointerTo(), "cst");
+  return Builder.CreatePointerCast(
+      remap(Tmp1), PointerType::getUnqual(ValTy->getContext()), "cst");
 }
----------------
barannikov88 wrote:
> Isn't this a no-op? And the one above?
> 
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`.


================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:193-194
         Value *BiasInst = Builder.Insert(OrigBiasInst->clone());
-        Addr = Builder.CreateIntToPtr(BiasInst, Ty->getPointerTo());
+        Addr = Builder.CreateIntToPtr(BiasInst,
+                                      PointerType::getUnqual(Ty->getContext()));
       }
----------------
barannikov88 wrote:
> Would that be correct?
> 
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())` .


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3389
     IRB.CreateStore(getCleanShadow(Ty),
-                    IRB.CreatePointerCast(ShadowPtr, Ty->getPointerTo()));
+                    IRB.CreatePointerCast(
+                        ShadowPtr, PointerType::getUnqual(Ty->getContext())));
----------------
barannikov88 wrote:
> This cast looks no-op.
Thank you for noticing this.
Removing the pointercast doesn't cause any regressions in `ninja check-llvm`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155232/new/

https://reviews.llvm.org/D155232



More information about the llvm-commits mailing list