[ConstantFolding] Avoid use of isNonIntegralPointerType() #159959

arichardson · 2025-09-21T00:32:30Z

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 #105735.

Created using spr 1.3.8-beta.1 [skip ci]

Created using spr 1.3.8-beta.1

llvmbot · 2025-09-21T00:33:03Z

@llvm/pr-subscribers-llvm-transforms

Author: Alexander Richardson (arichardson)

Changes

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 #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 --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
+}

llvmbot · 2025-09-21T00:33:04Z

@llvm/pr-subscribers-llvm-analysis

Author: Alexander Richardson (arichardson)

Changes

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 #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 --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
+}

arichardson · 2025-09-21T00:36:17Z

llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-nonintegral.ll

+
+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)


I'm not sure this really matters for CHERI, we could also emit the inttoptr (i16 -256 to ptr addrspace(1)) directly, but maybe it's cleaner to not attempt to perform any inttoptr based transforms?

nikic

LGTM

Created using spr 1.3.8-beta.1 [skip ci]

Created using spr 1.3.8-beta.1

llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-nonintegral.ll

krzysz00 · 2025-09-22T18:10:51Z

The transform looks correct to me

Created using spr 1.3.8-beta.1 [skip ci]

Created using spr 1.3.8-beta.1

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 llvm/llvm-project#105735. Reviewed By: nikic Pull Request: llvm/llvm-project#159959

arichardson added 2 commits September 20, 2025 17:32

[𝘀𝗽𝗿] changes to main this commit is based on

acd5695

Created using spr 1.3.8-beta.1 [skip ci]

[𝘀𝗽𝗿] initial version

cccf6db

Created using spr 1.3.8-beta.1

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Sep 21, 2025

arichardson requested review from arsenm, jrtc27, krzysz00, nikic and resistor September 21, 2025 00:33

arichardson commented Sep 21, 2025

View reviewed changes

arichardson requested a review from davidchisnall September 21, 2025 00:36

arichardson added the cheri Related to CHERI support label Sep 21, 2025

arichardson mentioned this pull request Sep 21, 2025

[DataLayout][LangRef] Split non-integral and unstable pointer properties #105735

Merged

nikic approved these changes Sep 21, 2025

View reviewed changes

arichardson added 4 commits September 21, 2025 14:49

[𝘀𝗽𝗿] changes introduced through rebase

e866587

Created using spr 1.3.8-beta.1 [skip ci]

fix unused variable

7005664

Created using spr 1.3.8-beta.1

fix typo in comment

690f308

Created using spr 1.3.8-beta.1

spelling

d18eb5e

Created using spr 1.3.8-beta.1

arsenm reviewed Sep 22, 2025

View reviewed changes

llvm/test/Transforms/InstSimplify/ConstProp/inttoptr-gep-nonintegral.ll Show resolved Hide resolved

arichardson added 2 commits September 23, 2025 11:58

[𝘀𝗽𝗿] changes introduced through rebase

6dd1ea4

Created using spr 1.3.8-beta.1 [skip ci]

add vector test cases

f705aa7

Created using spr 1.3.8-beta.1

arichardson changed the base branch from users/arichardson/spr/main.constantfolding-avoid-use-of-isnonintegralpointertype to main September 23, 2025 22:29

arichardson merged commit fd81bd8 into main Sep 23, 2025
13 of 15 checks passed

arichardson deleted the users/arichardson/spr/constantfolding-avoid-use-of-isnonintegralpointertype branch September 23, 2025 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ConstantFolding] Avoid use of isNonIntegralPointerType() #159959

[ConstantFolding] Avoid use of isNonIntegralPointerType() #159959

Uh oh!

arichardson commented Sep 21, 2025

Uh oh!

llvmbot commented Sep 21, 2025

Uh oh!

llvmbot commented Sep 21, 2025

Uh oh!

arichardson Sep 21, 2025

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

krzysz00 commented Sep 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[ConstantFolding] Avoid use of isNonIntegralPointerType() #159959

[ConstantFolding] Avoid use of isNonIntegralPointerType() #159959

Uh oh!

Conversation

arichardson commented Sep 21, 2025

Uh oh!

llvmbot commented Sep 21, 2025

Uh oh!

llvmbot commented Sep 21, 2025

Uh oh!

arichardson Sep 21, 2025

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krzysz00 commented Sep 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants