TargetLibraryInfo: Use pointer index size to determine getSizeTSize().

[resistor]

When using non-integral pointer types, such as on CHERI targets, size_t is equivalent
to the index size, which is allowed to be smaller than the size of the pointer.

[resistor]

Tagging @nzaghen specifically because of the changes in stdio-custom-dl.ll. The file was added by them in https://reviews.llvm.org/D68328

It's clear that the file is intended to test the opposite of the behavior being implemented here, but the reasoning behind that expectation is not clear to me.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Owen Anderson (resistor)

Changes

When using non-integral pointer types, such as on CHERI targets, size_t is equivalent
to the index size, which is allowed to be smaller than the size of the pointer.

---

Full diff: https://github.com/llvm/llvm-project/pull/118747.diff

3 Files Affected:

• (modified) llvm/lib/Analysis/TargetLibraryInfo.cpp (+3-6)
• (modified) llvm/test/Transforms/InstCombine/stdio-custom-dl.ll (+1-2)
• (modified) llvm/test/Transforms/MergeICmps/X86/distinct-index-width-crash.ll (+2-2)

diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index e0482b2b1ce025..b4bd53c24eecb0 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -1465,13 +1465,10 @@ unsigned TargetLibraryInfoImpl::getSizeTSize(const Module &M) const {
 
   // Historically LLVM assume that size_t has same size as intptr_t (hence
   // deriving the size from sizeof(int*) in address space zero). This should
-  // work for most targets. For future consideration: DataLayout also implement
-  // getIndexSizeInBits which might map better to size_t compared to
-  // getPointerSizeInBits. Hard coding address space zero here might be
-  // unfortunate as well. Maybe getDefaultGlobalsAddressSpace() or
-  // getAllocaAddrSpace() is better.
+  // work for most targets. For future consideration: Hard coding address space
+  // zero here might be unfortunate. Maybe getMaxIndexSizeInBits() is better.
   unsigned AddressSpace = 0;
-  return M.getDataLayout().getPointerSizeInBits(AddressSpace);
+  return M.getDataLayout().getIndexSizeInBits(AddressSpace);
 }
 
 TargetLibraryInfoWrapperPass::TargetLibraryInfoWrapperPass()
diff --git a/llvm/test/Transforms/InstCombine/stdio-custom-dl.ll b/llvm/test/Transforms/InstCombine/stdio-custom-dl.ll
index cc06be7e759d0c..44b702d8391225 100644
--- a/llvm/test/Transforms/InstCombine/stdio-custom-dl.ll
+++ b/llvm/test/Transforms/InstCombine/stdio-custom-dl.ll
@@ -8,11 +8,10 @@ target datalayout = "e-m:o-p:40:64:64:32-i64:64-f80:128-n8:16:32:64-S128"
 @.str.1 = private unnamed_addr constant [2 x i8] c"w\00", align 1
 @.str.2 = private unnamed_addr constant [4 x i8] c"str\00", align 1
 
-; Check fwrite is generated with arguments of ptr size, not index size
 define internal void @fputs_test_custom_dl() {
 ; CHECK-LABEL: @fputs_test_custom_dl(
 ; CHECK-NEXT:    [[CALL:%.*]] = call ptr @fopen(ptr nonnull @.str, ptr nonnull @.str.1)
-; CHECK-NEXT:    [[TMP1:%.*]] = call i40 @fwrite(ptr nonnull @.str.2, i40 3, i40 1, ptr [[CALL]])
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @fwrite(ptr nonnull @.str.2, i32 3, i32 1, ptr %call)
 ; CHECK-NEXT:    ret void
 ;
   %call = call ptr @fopen(ptr @.str, ptr @.str.1)
diff --git a/llvm/test/Transforms/MergeICmps/X86/distinct-index-width-crash.ll b/llvm/test/Transforms/MergeICmps/X86/distinct-index-width-crash.ll
index 7dce968ee9de0b..8ff7e95674f963 100644
--- a/llvm/test/Transforms/MergeICmps/X86/distinct-index-width-crash.ll
+++ b/llvm/test/Transforms/MergeICmps/X86/distinct-index-width-crash.ll
@@ -8,7 +8,7 @@ target triple = "x86_64"
 target datalayout = "e-p:64:64:64:32"
 
 ; Define a cunstom data layout that has index width < pointer width
-; and make sure that doesn't mreak anything
+; and make sure that doesn't break anything
 define void @fat_ptrs(ptr dereferenceable(16) %a, ptr dereferenceable(16) %b) {
 ; CHECK-LABEL: @fat_ptrs(
 ; CHECK-NEXT:  bb0:
@@ -16,7 +16,7 @@ define void @fat_ptrs(ptr dereferenceable(16) %a, ptr dereferenceable(16) %b) {
 ; CHECK-NEXT:    [[PTR_B1:%.*]] = getelementptr inbounds [2 x i64], ptr [[B:%.*]], i32 0, i32 1
 ; CHECK-NEXT:    br label %"bb1+bb2"
 ; CHECK:       "bb1+bb2":
-; CHECK-NEXT:    [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[A]], ptr [[B]], i64 16)
+; CHECK-NEXT:    [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[A]], ptr [[B]], i32 16)
 ; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i32 [[MEMCMP]], 0
 ; CHECK-NEXT:    br label [[BB3:%.*]]
 ; CHECK:       bb3:

[nikic]

This LGTM conceptually, but I think you do need to also switch calls in SLC to use getSizeTSize in places that currently use getIntPtrType. These values previously happened to be the same, but they won't be now (if the pointer and index size differ) and that will lead to assertion failures.

[resistor]

This LGTM conceptually, but I think you do need to also switch calls in SLC to use getSizeTSize in places that currently use getIntPtrType. These values previously happened to be the same, but they won't be now (if the pointer and index size differ) and that will lead to assertion failures.

I threaded it through SLC, PTAL

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[resistor]

Tagging @jrtc27 and @arichardson for CHERI-derived PRs.

[nikic]

LGTM

[s-barannikov]

LGTM as well