<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 23, 2021 at 10:18 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jun 21, 2021 at 12:25 PM Nikita Popov via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
><br>
> Author: Nikita Popov<br>
> Date: 2021-06-21T21:24:50+02:00<br>
> New Revision: d9f5d7b959de36085944d4a99a73f3053f953796<br>
><br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/d9f5d7b959de36085944d4a99a73f3053f953796" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/d9f5d7b959de36085944d4a99a73f3053f953796</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/d9f5d7b959de36085944d4a99a73f3053f953796.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/d9f5d7b959de36085944d4a99a73f3053f953796.diff</a><br>
><br>
> LOG: [InstCombine] Extract bitcast -> gep transform<br>
><br>
> Move this into a separate function, to make sure that early<br>
> returns do not accidentally skip other transforms. There is<br>
> already one isSized() check that could run into this issue,<br>
> thus this change is not strictly NFC.<br>
<br>
Can/should this be tested, then?<br></blockquote><div><br></div><div>Yeah :) I ended up reverting this and reapplying with a test in <a href="https://github.com/llvm/llvm-project/commit/e2c2124a4b5bad9cf2a1e23a6aef1b2ad753f504">https://github.com/llvm/llvm-project/commit/e2c2124a4b5bad9cf2a1e23a6aef1b2ad753f504</a>.</div><div><br></div><div>Nikita<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
><br>
> Added:<br>
><br>
><br>
> Modified:<br>
> llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
><br>
> Removed:<br>
><br>
><br>
><br>
> ################################################################################<br>
> diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
> index 9b38eb6ab24d..dcfff4552f11 100644<br>
> --- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
> +++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
> @@ -2603,6 +2603,52 @@ Instruction *InstCombinerImpl::optimizeBitCastFromPhi(CastInst &CI,<br>
> return RetVal;<br>
> }<br>
><br>
> +static Instruction *convertBitCastToGEP(BitCastInst &CI, IRBuilderBase &Builder,<br>
> + const DataLayout &DL) {<br>
> + Value *Src = CI.getOperand(0);<br>
> + PointerType *SrcPTy = cast<PointerType>(Src->getType());<br>
> + PointerType *DstPTy = cast<PointerType>(CI.getType());<br>
> + Type *DstElTy = DstPTy->getElementType();<br>
> + Type *SrcElTy = SrcPTy->getElementType();<br>
> +<br>
> + // When the type pointed to is not sized the cast cannot be<br>
> + // turned into a gep.<br>
> + if (!SrcElTy->isSized())<br>
> + return nullptr;<br>
> +<br>
> + // If the source and destination are pointers, and this cast is equivalent<br>
> + // to a getelementptr X, 0, 0, 0... turn it into the appropriate gep.<br>
> + // This can enhance SROA and other transforms that want type-safe pointers.<br>
> + unsigned NumZeros = 0;<br>
> + while (SrcElTy && SrcElTy != DstElTy) {<br>
> + SrcElTy = GetElementPtrInst::getTypeAtIndex(SrcElTy, (uint64_t)0);<br>
> + ++NumZeros;<br>
> + }<br>
> +<br>
> + // If we found a path from the src to dest, create the getelementptr now.<br>
> + if (SrcElTy == DstElTy) {<br>
> + SmallVector<Value *, 8> Idxs(NumZeros + 1, Builder.getInt32(0));<br>
> + GetElementPtrInst *GEP =<br>
> + GetElementPtrInst::Create(SrcPTy->getElementType(), Src, Idxs);<br>
> +<br>
> + // If the source pointer is dereferenceable, then assume it points to an<br>
> + // allocated object and apply "inbounds" to the GEP.<br>
> + bool CanBeNull, CanBeFreed;<br>
> + if (Src->getPointerDereferenceableBytes(DL, CanBeNull, CanBeFreed)) {<br>
> + // In a non-default address space (not 0), a null pointer can not be<br>
> + // assumed inbounds, so ignore that case (dereferenceable_or_null).<br>
> + // The reason is that 'null' is not treated<br>
> diff erently in these address<br>
> + // spaces, and we consequently ignore the 'gep inbounds' special case<br>
> + // for 'null' which allows 'inbounds' on 'null' if the indices are<br>
> + // zeros.<br>
> + if (SrcPTy->getAddressSpace() == 0 || !CanBeNull)<br>
> + GEP->setIsInBounds();<br>
> + }<br>
> + return GEP;<br>
> + }<br>
> + return nullptr;<br>
> +}<br>
> +<br>
> Instruction *InstCombinerImpl::visitBitCast(BitCastInst &CI) {<br>
> // If the operands are integer typed then apply the integer transforms,<br>
> // otherwise just apply the common ones.<br>
> @@ -2616,11 +2662,6 @@ Instruction *InstCombinerImpl::visitBitCast(BitCastInst &CI) {<br>
> return replaceInstUsesWith(CI, Src);<br>
><br>
> if (isa<PointerType>(SrcTy) && isa<PointerType>(DestTy)) {<br>
> - PointerType *SrcPTy = cast<PointerType>(SrcTy);<br>
> - PointerType *DstPTy = cast<PointerType>(DestTy);<br>
> - Type *DstElTy = DstPTy->getElementType();<br>
> - Type *SrcElTy = SrcPTy->getElementType();<br>
> -<br>
> // If we are casting a alloca to a pointer to a type of the same<br>
> // size, rewrite the allocation instruction to allocate the "right" type.<br>
> // There is no need to modify malloc calls because it is their bitcast that<br>
> @@ -2629,43 +2670,8 @@ Instruction *InstCombinerImpl::visitBitCast(BitCastInst &CI) {<br>
> if (Instruction *V = PromoteCastOfAllocation(CI, *AI))<br>
> return V;<br>
><br>
> - // When the type pointed to is not sized the cast cannot be<br>
> - // turned into a gep.<br>
> - Type *PointeeType =<br>
> - cast<PointerType>(Src->getType()->getScalarType())->getElementType();<br>
> - if (!PointeeType->isSized())<br>
> - return nullptr;<br>
> -<br>
> - // If the source and destination are pointers, and this cast is equivalent<br>
> - // to a getelementptr X, 0, 0, 0... turn it into the appropriate gep.<br>
> - // This can enhance SROA and other transforms that want type-safe pointers.<br>
> - unsigned NumZeros = 0;<br>
> - while (SrcElTy && SrcElTy != DstElTy) {<br>
> - SrcElTy = GetElementPtrInst::getTypeAtIndex(SrcElTy, (uint64_t)0);<br>
> - ++NumZeros;<br>
> - }<br>
> -<br>
> - // If we found a path from the src to dest, create the getelementptr now.<br>
> - if (SrcElTy == DstElTy) {<br>
> - SmallVector<Value *, 8> Idxs(NumZeros + 1, Builder.getInt32(0));<br>
> - GetElementPtrInst *GEP =<br>
> - GetElementPtrInst::Create(SrcPTy->getElementType(), Src, Idxs);<br>
> -<br>
> - // If the source pointer is dereferenceable, then assume it points to an<br>
> - // allocated object and apply "inbounds" to the GEP.<br>
> - bool CanBeNull, CanBeFreed;<br>
> - if (Src->getPointerDereferenceableBytes(DL, CanBeNull, CanBeFreed)) {<br>
> - // In a non-default address space (not 0), a null pointer can not be<br>
> - // assumed inbounds, so ignore that case (dereferenceable_or_null).<br>
> - // The reason is that 'null' is not treated<br>
> diff erently in these address<br>
> - // spaces, and we consequently ignore the 'gep inbounds' special case<br>
> - // for 'null' which allows 'inbounds' on 'null' if the indices are<br>
> - // zeros.<br>
> - if (SrcPTy->getAddressSpace() == 0 || !CanBeNull)<br>
> - GEP->setIsInBounds();<br>
> - }<br>
> - return GEP;<br>
> - }<br>
> + if (Instruction *I = convertBitCastToGEP(CI, Builder, DL))<br>
> + return I;<br>
> }<br>
><br>
> if (FixedVectorType *DestVTy = dyn_cast<FixedVectorType>(DestTy)) {<br>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>