New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IR] Require index width to be ule pointer width #70015
Conversation
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-llvm-analysis Author: Nikita Popov (nikic) ChangesI don't think there is a use case for having an index type that is wider than the pointer type, and I'm not entirely clear what semantics this would even have. Also clarify the GEP semantics to explicitly say that it only affects the bits up to the index type width. Full diff: https://github.com/llvm/llvm-project/pull/70015.diff 5 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index e4625cae9fc53e1..3d26709a1eb17c4 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -2883,7 +2883,8 @@ as follows:
This specifies the *size* of a pointer and its ``<abi>`` and
``<pref>``\erred alignments for address space ``n``. ``<pref>`` is optional
and defaults to ``<abi>``. The fourth parameter ``<idx>`` is the size of the
- index that used for address calculation. If not
+ index that used for address calculation, which must be less than or equal
+ to the pointer size. If not
specified, the default index size is equal to the pointer size. All sizes
are in bits. The address space, ``n``, is optional, and if not specified,
denotes the default address space 0. The value of ``n`` must be
@@ -11061,13 +11062,16 @@ These rules are based on the assumption that no allocated object may cross
the unsigned address space boundary, and no allocated object may be larger
than half the pointer index type space.
-If the ``inbounds`` keyword is not present, the offsets are added to the
-base address with silently-wrapping two's complement arithmetic. If the
-offsets have a different width from the pointer's index type, they are
-sign-extended or truncated to the width of the pointer's index type. The result
-value of the ``getelementptr`` may be outside the object pointed to by the base
-pointer. The result value may not necessarily be used to access memory
-though, even if it happens to point into allocated storage. See the
+If the ``inbounds`` keyword is not present, the offsets are first
+sign-extended or truncated to the pointer's index type, and then added
+to the low bits of the base address up to the index type width, with
+silently-wrapping two's complement arithmetic. If the pointer size is larger
+than the index size, this means that the bits outside the index type width
+will not be affected.
+
+The result value of the ``getelementptr`` may be outside the object pointed
+to by the base pointer. The result value may not necessarily be used to access
+memory though, even if it happens to point into allocated storage. See the
:ref:`Pointer Aliasing Rules <pointeraliasing>` section for more
information.
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index d324c4b488f7294..c8ef232082554bf 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -649,6 +649,8 @@ Error DataLayout::setPointerAlignmentInBits(uint32_t AddrSpace, Align ABIAlign,
if (PrefAlign < ABIAlign)
return reportError(
"Preferred alignment cannot be less than the ABI alignment");
+ if (IndexBitWidth > TypeBitWidth)
+ return reportError("Index width cannot be larger than pointer width");
auto I = lower_bound(Pointers, AddrSpace,
[](const PointerAlignElem &A, uint32_t AddressSpace) {
diff --git a/llvm/test/Analysis/ScalarEvolution/ptrtoint-constantexpr-loop.ll b/llvm/test/Analysis/ScalarEvolution/ptrtoint-constantexpr-loop.ll
index ca39b0b40e31614..31a6a6ec0023e58 100644
--- a/llvm/test/Analysis/ScalarEvolution/ptrtoint-constantexpr-loop.ll
+++ b/llvm/test/Analysis/ScalarEvolution/ptrtoint-constantexpr-loop.ll
@@ -2,7 +2,6 @@
; RUN: opt < %s --data-layout="p:64:64:64:64" -S -disable-output "-passes=print<scalar-evolution>" 2>&1 | FileCheck --check-prefixes=PTR64_IDX64 %s
; RUN: opt < %s --data-layout="p:64:64:64:32" -S -disable-output "-passes=print<scalar-evolution>" 2>&1 | FileCheck --check-prefixes=PTR64_IDX32 %s
; RUN: opt < %s --data-layout="p:16:16:16:16" -S -disable-output "-passes=print<scalar-evolution>" 2>&1 | FileCheck --check-prefixes=PTR16_IDX16 %s
-; RUN: opt < %s --data-layout="p:16:16:16:32" -S -disable-output "-passes=print<scalar-evolution>" 2>&1 | FileCheck --check-prefixes=PTR16_IDX32 %s
@global = external hidden global [0 x i8]
@@ -63,24 +62,6 @@ define hidden ptr @trunc_ptr_to_i64(ptr %arg, ptr %arg10) {
; PTR16_IDX16-NEXT: Loop %bb11: Unpredictable symbolic max backedge-taken count.
; PTR16_IDX16-NEXT: Loop %bb11: Unpredictable predicated backedge-taken count.
;
-; PTR16_IDX32-LABEL: 'trunc_ptr_to_i64'
-; PTR16_IDX32-NEXT: Classifying expressions for: @trunc_ptr_to_i64
-; PTR16_IDX32-NEXT: %tmp = phi i32 [ 0, %bb ], [ %tmp18, %bb17 ]
-; PTR16_IDX32-NEXT: --> {0,+,2}<%bb11> U: [0,-1) S: [-2147483648,2147483647) Exits: <<Unknown>> LoopDispositions: { %bb11: Computable }
-; PTR16_IDX32-NEXT: %tmp12 = getelementptr i8, ptr %arg, i64 ptrtoint (ptr @global to i64)
-; PTR16_IDX32-NEXT: --> ((trunc i64 ptrtoint (ptr @global to i64) to i32) + %arg) U: [0,131071) S: [0,131071) Exits: ((trunc i64 ptrtoint (ptr @global to i64) to i32) + %arg) LoopDispositions: { %bb11: Invariant }
-; PTR16_IDX32-NEXT: %tmp13 = bitcast ptr %tmp12 to ptr
-; PTR16_IDX32-NEXT: --> ((trunc i64 ptrtoint (ptr @global to i64) to i32) + %arg) U: [0,131071) S: [0,131071) Exits: ((trunc i64 ptrtoint (ptr @global to i64) to i32) + %arg) LoopDispositions: { %bb11: Invariant }
-; PTR16_IDX32-NEXT: %tmp14 = load i32, ptr %tmp13, align 4
-; PTR16_IDX32-NEXT: --> %tmp14 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb11: Variant }
-; PTR16_IDX32-NEXT: %tmp18 = add i32 %tmp, 2
-; PTR16_IDX32-NEXT: --> {2,+,2}<%bb11> U: [0,-1) S: [-2147483648,2147483647) Exits: <<Unknown>> LoopDispositions: { %bb11: Computable }
-; PTR16_IDX32-NEXT: Determining loop execution counts for: @trunc_ptr_to_i64
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable constant max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable symbolic max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable predicated backedge-taken count.
-;
bb:
br label %bb11
@@ -154,24 +135,6 @@ define hidden ptr @trunc_ptr_to_i32(ptr %arg, ptr %arg10) {
; PTR16_IDX16-NEXT: Loop %bb11: Unpredictable symbolic max backedge-taken count.
; PTR16_IDX16-NEXT: Loop %bb11: Unpredictable predicated backedge-taken count.
;
-; PTR16_IDX32-LABEL: 'trunc_ptr_to_i32'
-; PTR16_IDX32-NEXT: Classifying expressions for: @trunc_ptr_to_i32
-; PTR16_IDX32-NEXT: %tmp = phi i32 [ 0, %bb ], [ %tmp18, %bb17 ]
-; PTR16_IDX32-NEXT: --> {0,+,2}<%bb11> U: [0,-1) S: [-2147483648,2147483647) Exits: <<Unknown>> LoopDispositions: { %bb11: Computable }
-; PTR16_IDX32-NEXT: %tmp12 = getelementptr i8, ptr %arg, i32 ptrtoint (ptr @global to i32)
-; PTR16_IDX32-NEXT: --> (ptrtoint (ptr @global to i32) + %arg) U: [0,131071) S: [0,131071) Exits: (ptrtoint (ptr @global to i32) + %arg) LoopDispositions: { %bb11: Invariant }
-; PTR16_IDX32-NEXT: %tmp13 = bitcast ptr %tmp12 to ptr
-; PTR16_IDX32-NEXT: --> (ptrtoint (ptr @global to i32) + %arg) U: [0,131071) S: [0,131071) Exits: (ptrtoint (ptr @global to i32) + %arg) LoopDispositions: { %bb11: Invariant }
-; PTR16_IDX32-NEXT: %tmp14 = load i32, ptr %tmp13, align 4
-; PTR16_IDX32-NEXT: --> %tmp14 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb11: Variant }
-; PTR16_IDX32-NEXT: %tmp18 = add i32 %tmp, 2
-; PTR16_IDX32-NEXT: --> {2,+,2}<%bb11> U: [0,-1) S: [-2147483648,2147483647) Exits: <<Unknown>> LoopDispositions: { %bb11: Computable }
-; PTR16_IDX32-NEXT: Determining loop execution counts for: @trunc_ptr_to_i32
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable constant max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable symbolic max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable predicated backedge-taken count.
-;
bb:
br label %bb11
@@ -245,24 +208,6 @@ define hidden ptr @trunc_ptr_to_i128(ptr %arg, ptr %arg10) {
; PTR16_IDX16-NEXT: Loop %bb11: Unpredictable symbolic max backedge-taken count.
; PTR16_IDX16-NEXT: Loop %bb11: Unpredictable predicated backedge-taken count.
;
-; PTR16_IDX32-LABEL: 'trunc_ptr_to_i128'
-; PTR16_IDX32-NEXT: Classifying expressions for: @trunc_ptr_to_i128
-; PTR16_IDX32-NEXT: %tmp = phi i32 [ 0, %bb ], [ %tmp18, %bb17 ]
-; PTR16_IDX32-NEXT: --> {0,+,2}<%bb11> U: [0,-1) S: [-2147483648,2147483647) Exits: <<Unknown>> LoopDispositions: { %bb11: Computable }
-; PTR16_IDX32-NEXT: %tmp12 = getelementptr i8, ptr %arg, i128 ptrtoint (ptr @global to i128)
-; PTR16_IDX32-NEXT: --> ((trunc i128 ptrtoint (ptr @global to i128) to i32) + %arg) U: [0,131071) S: [0,131071) Exits: ((trunc i128 ptrtoint (ptr @global to i128) to i32) + %arg) LoopDispositions: { %bb11: Invariant }
-; PTR16_IDX32-NEXT: %tmp13 = bitcast ptr %tmp12 to ptr
-; PTR16_IDX32-NEXT: --> ((trunc i128 ptrtoint (ptr @global to i128) to i32) + %arg) U: [0,131071) S: [0,131071) Exits: ((trunc i128 ptrtoint (ptr @global to i128) to i32) + %arg) LoopDispositions: { %bb11: Invariant }
-; PTR16_IDX32-NEXT: %tmp14 = load i32, ptr %tmp13, align 4
-; PTR16_IDX32-NEXT: --> %tmp14 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb11: Variant }
-; PTR16_IDX32-NEXT: %tmp18 = add i32 %tmp, 2
-; PTR16_IDX32-NEXT: --> {2,+,2}<%bb11> U: [0,-1) S: [-2147483648,2147483647) Exits: <<Unknown>> LoopDispositions: { %bb11: Computable }
-; PTR16_IDX32-NEXT: Determining loop execution counts for: @trunc_ptr_to_i128
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable constant max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable symbolic max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb11: Unpredictable predicated backedge-taken count.
-;
bb:
br label %bb11
@@ -319,18 +264,6 @@ define void @zext_ptr_to_i32(i32 %arg, i32 %arg6) {
; PTR16_IDX16-NEXT: Loop %bb7: Unpredictable symbolic max backedge-taken count.
; PTR16_IDX16-NEXT: Loop %bb7: Unpredictable predicated backedge-taken count.
;
-; PTR16_IDX32-LABEL: 'zext_ptr_to_i32'
-; PTR16_IDX32-NEXT: Classifying expressions for: @zext_ptr_to_i32
-; PTR16_IDX32-NEXT: %tmp = sub i32 %arg, ptrtoint (ptr @global to i32)
-; PTR16_IDX32-NEXT: --> ((-1 * ptrtoint (ptr @global to i32))<nsw> + %arg) U: full-set S: full-set Exits: ((-1 * ptrtoint (ptr @global to i32))<nsw> + %arg) LoopDispositions: { %bb7: Invariant }
-; PTR16_IDX32-NEXT: %tmp9 = select i1 %tmp8, i16 0, i16 1
-; PTR16_IDX32-NEXT: --> %tmp9 U: [0,2) S: [0,2) Exits: <<Unknown>> LoopDispositions: { %bb7: Variant }
-; PTR16_IDX32-NEXT: Determining loop execution counts for: @zext_ptr_to_i32
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable constant max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable symbolic max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable predicated backedge-taken count.
-;
bb:
br label %bb7
@@ -382,18 +315,6 @@ define void @sext_to_i32(i32 %arg, i32 %arg6) {
; PTR16_IDX16-NEXT: Loop %bb7: Unpredictable symbolic max backedge-taken count.
; PTR16_IDX16-NEXT: Loop %bb7: Unpredictable predicated backedge-taken count.
;
-; PTR16_IDX32-LABEL: 'sext_to_i32'
-; PTR16_IDX32-NEXT: Classifying expressions for: @sext_to_i32
-; PTR16_IDX32-NEXT: %tmp = sub i32 %arg, sext (i16 ptrtoint (ptr @global to i16) to i32)
-; PTR16_IDX32-NEXT: --> ((-1 * (sext i16 ptrtoint (ptr @global to i16) to i32))<nsw> + %arg) U: full-set S: full-set Exits: ((-1 * (sext i16 ptrtoint (ptr @global to i16) to i32))<nsw> + %arg) LoopDispositions: { %bb7: Invariant }
-; PTR16_IDX32-NEXT: %tmp9 = select i1 %tmp8, i16 0, i16 1
-; PTR16_IDX32-NEXT: --> %tmp9 U: [0,2) S: [0,2) Exits: <<Unknown>> LoopDispositions: { %bb7: Variant }
-; PTR16_IDX32-NEXT: Determining loop execution counts for: @sext_to_i32
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable constant max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable symbolic max backedge-taken count.
-; PTR16_IDX32-NEXT: Loop %bb7: Unpredictable predicated backedge-taken count.
-;
bb:
br label %bb7
@@ -463,24 +384,6 @@ define i64 @sext_like_noop(i32 %n) {
; PTR16_IDX16-NEXT: Predicates:
; PTR16_IDX16: Loop %for.body: Trip multiple is 1
;
-; PTR16_IDX32-LABEL: 'sext_like_noop'
-; PTR16_IDX32-NEXT: Classifying expressions for: @sext_like_noop
-; PTR16_IDX32-NEXT: %ii = sext i32 %i to i64
-; PTR16_IDX32-NEXT: --> (sext i32 {1,+,1}<nuw><%for.body> to i64) U: [-2147483648,2147483648) S: [-2147483648,2147483648) --> (-1 + (zext i32 ptrtoint (ptr @sext_like_noop to i32) to i64))<nsw> U: [-1,65535) S: [-1,65535)
-; PTR16_IDX32-NEXT: %div = sdiv i64 55555, %ii
-; PTR16_IDX32-NEXT: --> %div U: full-set S: full-set
-; PTR16_IDX32-NEXT: %i = phi i32 [ %inc, %for.body ], [ 1, %entry ]
-; PTR16_IDX32-NEXT: --> {1,+,1}<nuw><%for.body> U: [1,0) S: [1,0) Exits: (-1 + ptrtoint (ptr @sext_like_noop to i32))<nsw> LoopDispositions: { %for.body: Computable }
-; PTR16_IDX32-NEXT: %inc = add nuw i32 %i, 1
-; PTR16_IDX32-NEXT: --> {2,+,1}<nuw><%for.body> U: [2,0) S: [2,0) Exits: ptrtoint (ptr @sext_like_noop to i32) LoopDispositions: { %for.body: Computable }
-; PTR16_IDX32-NEXT: Determining loop execution counts for: @sext_like_noop
-; PTR16_IDX32-NEXT: Loop %for.body: backedge-taken count is (-2 + ptrtoint (ptr @sext_like_noop to i32))<nsw>
-; PTR16_IDX32-NEXT: Loop %for.body: constant max backedge-taken count is -1
-; PTR16_IDX32-NEXT: Loop %for.body: symbolic max backedge-taken count is (-2 + ptrtoint (ptr @sext_like_noop to i32))<nsw>
-; PTR16_IDX32-NEXT: Loop %for.body: Predicated backedge-taken count is (-2 + ptrtoint (ptr @sext_like_noop to i32))<nsw>
-; PTR16_IDX32-NEXT: Predicates:
-; PTR16_IDX32: Loop %for.body: Trip multiple is 1
-;
entry:
%cmp6 = icmp sgt i32 %n, 1
br label %for.body
diff --git a/llvm/test/Assembler/invalid-datalayout-index-size.ll b/llvm/test/Assembler/invalid-datalayout-index-size.ll
new file mode 100644
index 000000000000000..dc608cdd56a0405
--- /dev/null
+++ b/llvm/test/Assembler/invalid-datalayout-index-size.ll
@@ -0,0 +1,3 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+target datalayout = "p:64:64:64:128"
+; CHECK: Index width cannot be larger than pointer width
diff --git a/llvm/test/Transforms/InferAlignment/ptrmask.ll b/llvm/test/Transforms/InferAlignment/ptrmask.ll
index afab872d16d5eac..7fb0220e92b967e 100644
--- a/llvm/test/Transforms/InferAlignment/ptrmask.ll
+++ b/llvm/test/Transforms/InferAlignment/ptrmask.ll
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
; RUN: opt < %s -passes=infer-alignment -S | FileCheck %s
-target datalayout = "p1:64:64:64:32-p2:64:64:64:128"
+target datalayout = "p1:64:64:64:32"
; ------------------------------------------------------------------------------
; load instructions
@@ -88,18 +88,5 @@ define i8 @smaller_index_type(ptr addrspace(1) %ptr) {
ret i8 %load
}
-define i8 @larger_index_type(ptr addrspace(2) %ptr) {
-; CHECK-LABEL: define i8 @larger_index_type
-; CHECK-SAME: (ptr addrspace(2) [[PTR:%.*]]) {
-; CHECK-NEXT: [[PTR2:%.*]] = call ptr addrspace(2) @llvm.ptrmask.p2.i128(ptr addrspace(2) [[PTR]], i128 -4)
-; CHECK-NEXT: [[LOAD:%.*]] = load i8, ptr addrspace(2) [[PTR2]], align 4
-; CHECK-NEXT: ret i8 [[LOAD]]
-;
- %ptr2 = call ptr addrspace(2) @llvm.ptrmask.p2.i128(ptr addrspace(2) %ptr, i128 -4)
- %load = load i8, ptr addrspace(2) %ptr2, align 1
- ret i8 %load
-}
-
declare ptr @llvm.ptrmask.p0.i64(ptr, i64)
declare ptr addrspace(1) @llvm.ptrmask.p1.i32(ptr addrspace(1), i32)
-declare ptr addrspace(2) @llvm.ptrmask.p2.i128(ptr addrspace(2), i128)
|
llvm/docs/LangRef.rst
Outdated
value of the ``getelementptr`` may be outside the object pointed to by the base | ||
pointer. The result value may not necessarily be used to access memory | ||
though, even if it happens to point into allocated storage. See the | ||
If the ``inbounds`` keyword is not present, the offsets are first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this paragraph needs to be qualified by "If the inbounds
keyword is not present".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea here was that in the inbounds case, overflow can't happen anyway, so the semantics don't really matter.
I've now done a larger restructuring, and moved the discussion of the general GEP semantics at the start, before the special inbounds semantics are discussed.
I've also added a description of how the GEP offsets are calculated, because I don't think we explained that anywhere (and it's kinda important, especially the order of trunc/ext and multiplication, and which type size is used).
3a68c35
to
0330d3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wording LGTM but would be good to add a test for the new error.
I don't think there is a use case for having an index type that is wider than the pointer type, and I'm not entirely clear what semantics this would even have. Also clarify the GEP semantics to explicitly say that it only affects the bits up to the index type width.
0330d3c
to
71d7d24
Compare
The indices are first converted to offsets in the pointer's index type. If the | ||
currently indexed type is a struct type, the struct offset corresponding to the | ||
index is sign-extended or truncated to the pointer index type. Otherwise, the | ||
index itself is sign-extended or truncated, and then multiplied by the type | ||
allocation size (that is, the size rounded up to the ABI alignment) of the | ||
currently indexed type. | ||
|
||
The offsets are then added to the low bits of the base address up to the index | ||
type width, with silently-wrapping two's complement arithmetic. If the pointer | ||
size is larger than the index size, this means that the bits outside the index | ||
type width will not be affected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I don't think there is a use case for having an index type that is wider than the pointer type, and I'm not entirely clear what semantics this would even have. Also clarify the GEP semantics to explicitly say how they interact with the index type width.
I don't think there is a use case for having an index type that is wider than the pointer type, and I'm not entirely clear what semantics this would even have.
Also clarify the GEP semantics to explicitly say that it only affects the bits up to the index type width.