llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-analysis Author: Alexander Richardson (arichardson) <details> <summary>Changes</summary> Avoiding any new inttoptr is unnecessarily restrictive for "plain" non-integral pointers, but it is important for unstable pointers and pointers with external state. Fixes another test codegen regression from https://github.com/llvm/llvm-project/pull/105735. --- Full diff: https://github.com/llvm/llvm-project/pull/159959.diff 3 Files Affected: - (modified) llvm/lib/Analysis/ConstantFolding.cpp (+7-6) - (removed) llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-index-width.ll (-16) - (added) llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-nonintegral.ll (+75) ``````````diff diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp index a3b2e62a1b8ba..e4eac1892cfde 100755 --- a/llvm/lib/Analysis/ConstantFolding.cpp +++ b/llvm/lib/Analysis/ConstantFolding.cpp @@ -951,21 +951,22 @@ Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP, // If the base value for this address is a literal integer value, fold the // getelementptr to the resulting integer value casted to the pointer type. - APInt BasePtr(DL.getPointerTypeSizeInBits(Ptr->getType()), 0); + APInt BaseIntVal(DL.getPointerTypeSizeInBits(Ptr->getType()), 0); if (auto *CE = dyn_cast<ConstantExpr>(Ptr)) { if (CE->getOpcode() == Instruction::IntToPtr) { if (auto *Base = dyn_cast<ConstantInt>(CE->getOperand(0))) - BasePtr = Base->getValue().zextOrTrunc(BasePtr.getBitWidth()); + BaseIntVal = Base->getValue().zextOrTrunc(BaseIntVal.getBitWidth()); } } auto *PTy = cast<PointerType>(Ptr->getType()); - if ((Ptr->isNullValue() || BasePtr != 0) && - !DL.isNonIntegralPointerType(PTy)) { + if ((Ptr->isNullValue() || BaseIntVal != 0) && + !DL.shouldAvoidIntToPtr(Ptr->getType())) { + // If the index size is smaller than the pointer size, add to the low // bits only. - BasePtr.insertBits(BasePtr.trunc(BitWidth) + Offset, 0); - Constant *C = ConstantInt::get(Ptr->getContext(), BasePtr); + BaseIntVal.insertBits(BaseIntVal.trunc(BitWidth) + Offset, 0); + Constant *C = ConstantInt::get(Ptr->getContext(), BaseIntVal); return ConstantExpr::getIntToPtr(C, ResTy); } diff --git a/llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-index-width.ll b/llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-index-width.ll deleted file mode 100644 index 2049def9b59b7..0000000000000 --- a/llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-index-width.ll +++ /dev/null @@ -1,16 +0,0 @@ -; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 -; RUN: opt -S -passes=instsimplify < %s | FileCheck %s - -target datalayout = "p:16:16:16:8" - -; The GEP should only modify the low 8 bits of the pointer. -define ptr @test() { -; CHECK-LABEL: define ptr @test() { -; We need to use finer-grained DataLayout properties for non-integral pointers -; FIXME: Should be: ret ptr inttoptr (i16 -256 to ptr) -; CHECK-NEXT: ret ptr getelementptr (i8, ptr inttoptr (i16 -1 to ptr), i8 1) -; - %base = inttoptr i16 -1 to ptr - %gep = getelementptr i8, ptr %base, i8 1 - ret ptr %gep -} diff --git a/llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-nonintegral.ll b/llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-nonintegral.ll new file mode 100644 index 0000000000000..16980b54f93ad --- /dev/null +++ b/llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-nonintegral.ll @@ -0,0 +1,75 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 +; RUN: opt -S -passes=instsimplify < %s | FileCheck %s +;; Check that we do not create new inttoptr intstructions for unstable pointers +;; or pointers with external state (even if the values are all constants). +;; NOTE: for all but the zero address space, the GEP should only modify the low 8 bits of the pointer. +target datalayout = "p:16:16:16:16-p1:16:16:16:8-pu2:16:16:16:8-pe3:16:16:16:8" + +define ptr @test_null_base_normal() { +; CHECK-LABEL: define ptr @test_null_base_normal() { +; CHECK-NEXT: ret ptr inttoptr (i16 -2 to ptr) +; + %gep = getelementptr i8, ptr null, i8 -2 + ret ptr %gep +} +define ptr @test_inttoptr_base_normal() { +; CHECK-LABEL: define ptr @test_inttoptr_base_normal() { +; CHECK-NEXT: ret ptr null +; + %base = inttoptr i16 -1 to ptr + %gep = getelementptr i8, ptr %base, i8 1 + ret ptr %gep +} + +;; Transformation is fine for non-integral address space, but we can only change +;; the index bits (i8 -2 == i16 254) +define ptr addrspace(1) @test_null_base_nonintegral() { +; CHECK-LABEL: define ptr addrspace(1) @test_null_base_nonintegral() { +; CHECK-NEXT: ret ptr addrspace(1) inttoptr (i16 254 to ptr addrspace(1)) +; + %gep = getelementptr i8, ptr addrspace(1) null, i8 -2 + ret ptr addrspace(1) %gep +} +define ptr addrspace(1) @test_inttoptr_base_nonintegral() { +; CHECK-LABEL: define ptr addrspace(1) @test_inttoptr_base_nonintegral() { +; CHECK-NEXT: ret ptr addrspace(1) inttoptr (i16 -256 to ptr addrspace(1)) +; + %base = inttoptr i16 -1 to ptr addrspace(1) + %gep = getelementptr i8, ptr addrspace(1) %base, i8 1 + ret ptr addrspace(1) %gep +} + +;; For unstable pointers we should avoid any introduction of inttoptr +define ptr addrspace(2) @test_null_base_unstable() { +; CHECK-LABEL: define ptr addrspace(2) @test_null_base_unstable() { +; CHECK-NEXT: ret ptr addrspace(2) getelementptr (i8, ptr addrspace(2) null, i8 -2) +; + %gep = getelementptr i8, ptr addrspace(2) null, i8 -2 + ret ptr addrspace(2) %gep +} +define ptr addrspace(2) @test_inttoptr_base_unstable() { +; CHECK-LABEL: define ptr addrspace(2) @test_inttoptr_base_unstable() { +; CHECK-NEXT: ret ptr addrspace(2) getelementptr (i8, ptr addrspace(2) inttoptr (i16 -1 to ptr addrspace(2)), i8 1) +; + %base = inttoptr i16 -1 to ptr addrspace(2) + %gep = getelementptr i8, ptr addrspace(2) %base, i8 1 + ret ptr addrspace(2) %gep +} + +;; The same is true for pointers with external state: no new inttoptr +define ptr addrspace(3) @test_null_base_external() { +; CHECK-LABEL: define ptr addrspace(3) @test_null_base_external() { +; CHECK-NEXT: ret ptr addrspace(3) getelementptr (i8, ptr addrspace(3) null, i8 -2) +; + %gep = getelementptr i8, ptr addrspace(3) null, i8 -2 + ret ptr addrspace(3) %gep +} + +define ptr addrspace(3) @test_inttoptr_base_external() { +; CHECK-LABEL: define ptr addrspace(3) @test_inttoptr_base_external() { +; CHECK-NEXT: ret ptr addrspace(3) getelementptr (i8, ptr addrspace(3) inttoptr (i16 -1 to ptr addrspace(3)), i8 1) +; + %base = inttoptr i16 -1 to ptr addrspace(3) + %gep = getelementptr i8, ptr addrspace(3) %base, i8 1 + ret ptr addrspace(3) %gep +} `````````` </details> https://github.com/llvm/llvm-project/pull/159959 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
