[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
nikic wrote: @dtcxzyw Auto-upgrade is only for bitcode files, we usually do not upgrade IR files. Can you regenerate the inputs with the new clang version? https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
dtcxzyw wrote: > This is a very niche feature, and I don't think trying to upgrade it is > worthwhile. It exists in many real-world applications. If you are not willing to implement the upgrader, I will do this for the original IR files in my benchmark :) https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
dtcxzyw wrote:
> bin/opt: ../../llvm-opt-benchmark/bench/icu/original/servlkf.ll:776:98:
> error: expected ')' in constantexpr
store ptr getelementptr inbounds ({ [11 x ptr] }, ptr
@_ZTVN6icu_7516LocaleKeyFactoryE, i32 0, inrange i32 0, i32 2), ptr %this1,
align 8
@nikic Do we need an auto-upgrader?
https://github.com/llvm/llvm-project/pull/84341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
https://github.com/nikic closed https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
https://github.com/nhaehnle approved this pull request. https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
@@ -47,28 +47,66 @@ static bool splitGlobal(GlobalVariable &GV) {
if (!Init)
return false;
- // Verify that each user of the global is an inrange getelementptr constant.
- // From this it follows that any loads from or stores to that global must use
- // a pointer derived from an inrange getelementptr constant, which is
- // sufficient to allow us to apply the splitting transform.
+ const DataLayout &DL = GV.getParent()->getDataLayout();
+ const StructLayout *SL = DL.getStructLayout(Init->getType());
+ ArrayRef MemberOffsets = SL->getMemberOffsets();
+ unsigned IndexWidth = DL.getIndexTypeSizeInBits(GV.getType());
+
+ // Verify that each user of the global is an inrange getelementptr constant,
+ // and collect information on how it relates to the global.
+ struct GEPInfo {
+GEPOperator *GEP;
+unsigned MemberIndex;
+APInt MemberRelativeOffset;
+
+GEPInfo(GEPOperator *GEP, unsigned MemberIndex, APInt MemberRelativeOffset)
+: GEP(GEP), MemberIndex(MemberIndex),
+ MemberRelativeOffset(std::move(MemberRelativeOffset)) {}
+ };
+ SmallVector Infos;
for (User *U : GV.users()) {
-if (!isa(U))
+auto *GEP = dyn_cast(U);
+if (!GEP)
return false;
-auto *GEP = dyn_cast(U);
-if (!GEP || !GEP->getInRangeIndex() || *GEP->getInRangeIndex() != 1 ||
-!isa(GEP->getOperand(1)) ||
-!cast(GEP->getOperand(1))->isZero() ||
-!isa(GEP->getOperand(2)))
+std::optional InRange = GEP->getInRange();
+if (!InRange)
+ return false;
+
+APInt Offset(IndexWidth, 0);
+if (!GEP->accumulateConstantOffset(DL, Offset))
+ return false;
+
+// Determine source-relative inrange.
+ConstantRange SrcInRange = InRange->sextOrTrunc(IndexWidth).add(Offset);
+
+// Check that the GEP offset is in the range (treating upper bound as
+// inclusive here).
+if (!SrcInRange.contains(Offset) && SrcInRange.getUpper() != Offset)
+ return false;
+
+// Find which struct member the range corresponds to.
+if (SrcInRange.getLower().uge(SL->getSizeInBytes()))
return false;
+
+unsigned MemberIndex =
+SL->getElementContainingOffset(SrcInRange.getLower().getZExtValue());
nhaehnle wrote:
You're right, I missed that it's an unsigned comparison.
https://github.com/llvm/llvm-project/pull/84341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
@@ -47,28 +47,66 @@ static bool splitGlobal(GlobalVariable &GV) {
if (!Init)
return false;
- // Verify that each user of the global is an inrange getelementptr constant.
- // From this it follows that any loads from or stores to that global must use
- // a pointer derived from an inrange getelementptr constant, which is
- // sufficient to allow us to apply the splitting transform.
+ const DataLayout &DL = GV.getParent()->getDataLayout();
+ const StructLayout *SL = DL.getStructLayout(Init->getType());
+ ArrayRef MemberOffsets = SL->getMemberOffsets();
+ unsigned IndexWidth = DL.getIndexTypeSizeInBits(GV.getType());
+
+ // Verify that each user of the global is an inrange getelementptr constant,
+ // and collect information on how it relates to the global.
+ struct GEPInfo {
+GEPOperator *GEP;
+unsigned MemberIndex;
+APInt MemberRelativeOffset;
+
+GEPInfo(GEPOperator *GEP, unsigned MemberIndex, APInt MemberRelativeOffset)
+: GEP(GEP), MemberIndex(MemberIndex),
+ MemberRelativeOffset(std::move(MemberRelativeOffset)) {}
+ };
+ SmallVector Infos;
for (User *U : GV.users()) {
-if (!isa(U))
+auto *GEP = dyn_cast(U);
+if (!GEP)
return false;
-auto *GEP = dyn_cast(U);
-if (!GEP || !GEP->getInRangeIndex() || *GEP->getInRangeIndex() != 1 ||
-!isa(GEP->getOperand(1)) ||
-!cast(GEP->getOperand(1))->isZero() ||
-!isa(GEP->getOperand(2)))
+std::optional InRange = GEP->getInRange();
+if (!InRange)
+ return false;
+
+APInt Offset(IndexWidth, 0);
+if (!GEP->accumulateConstantOffset(DL, Offset))
+ return false;
+
+// Determine source-relative inrange.
+ConstantRange SrcInRange = InRange->sextOrTrunc(IndexWidth).add(Offset);
+
+// Check that the GEP offset is in the range (treating upper bound as
+// inclusive here).
+if (!SrcInRange.contains(Offset) && SrcInRange.getUpper() != Offset)
+ return false;
+
+// Find which struct member the range corresponds to.
+if (SrcInRange.getLower().uge(SL->getSizeInBytes()))
return false;
+
+unsigned MemberIndex =
+SL->getElementContainingOffset(SrcInRange.getLower().getZExtValue());
nikic wrote:
I think this is already implied by the check directly before this (note that it
uses uge rather than sge, so will also reject negative numbers -- unless the
struct size becomes negative, of course...).
https://github.com/llvm/llvm-project/pull/84341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
@@ -23,24 +23,26 @@ @PR23753_b = global ptr getelementptr (i8, ptr @PR23753_a, i64 ptrtoint (ptr @PR23753_a to i64)) ; CHECK: @PR23753_b = global ptr getelementptr (i8, ptr @PR23753_a, i64 ptrtoint (ptr @PR23753_a to i64)) -; Verify that inrange on an index inhibits over-indexed getelementptr folding. +; Verify that inrange doesn't inhibit over-indexed getelementptr folding, +; but does inhibit combining two GEPs where the inner one has inrange (this +; will be done when DataLayout is available instead). @nestedarray = global [2 x [4 x ptr]] zeroinitializer -; CHECK: @nestedarray.1 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, inrange i32 0, i64 1, i32 0) -@nestedarray.1 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, inrange i32 0, i32 0, i32 4) +; CHECK: @nestedarray.1 = alias ptr, getelementptr inbounds inrange(-32, 32) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i64 1, i32 0) +@nestedarray.1 = alias ptr, getelementptr inbounds inrange(-32, 32) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i32 0, i32 4) -; CHECK: @nestedarray.2 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0, i32 4) -@nestedarray.2 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0, i32 4) +; CHECK: @nestedarray.2 = alias ptr, getelementptr inbounds inrange(0, 1) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i64 1, i32 0) +@nestedarray.2 = alias ptr, getelementptr inbounds inrange(0, 1) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i32 0, i32 4) -; CHECK: @nestedarray.3 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0) -@nestedarray.3 = alias ptr, getelementptr inbounds ([4 x ptr], ptr getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0), i32 0, i32 0) +; CHECK: @nestedarray.3 = alias ptr, getelementptr inbounds inrange(0, 4) ([4 x ptr], ptr @nestedarray, i32 0, i32 0) +@nestedarray.3 = alias ptr, getelementptr inbounds inrange(0, 4) ([4 x ptr], ptr getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i32 0), i32 0, i32 0) nikic wrote: These kinds of all zero GEPs usually get folded away during constant expression construction. However, they *are* retained if doing so would lose an inrange attribute. https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
@@ -23,24 +23,26 @@ @PR23753_b = global ptr getelementptr (i8, ptr @PR23753_a, i64 ptrtoint (ptr @PR23753_a to i64)) ; CHECK: @PR23753_b = global ptr getelementptr (i8, ptr @PR23753_a, i64 ptrtoint (ptr @PR23753_a to i64)) -; Verify that inrange on an index inhibits over-indexed getelementptr folding. +; Verify that inrange doesn't inhibit over-indexed getelementptr folding, +; but does inhibit combining two GEPs where the inner one has inrange (this +; will be done when DataLayout is available instead). @nestedarray = global [2 x [4 x ptr]] zeroinitializer -; CHECK: @nestedarray.1 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, inrange i32 0, i64 1, i32 0) -@nestedarray.1 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, inrange i32 0, i32 0, i32 4) +; CHECK: @nestedarray.1 = alias ptr, getelementptr inbounds inrange(-32, 32) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i64 1, i32 0) +@nestedarray.1 = alias ptr, getelementptr inbounds inrange(-32, 32) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i32 0, i32 4) -; CHECK: @nestedarray.2 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0, i32 4) -@nestedarray.2 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0, i32 4) +; CHECK: @nestedarray.2 = alias ptr, getelementptr inbounds inrange(0, 1) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i64 1, i32 0) +@nestedarray.2 = alias ptr, getelementptr inbounds inrange(0, 1) ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i32 0, i32 4) -; CHECK: @nestedarray.3 = alias ptr, getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0) -@nestedarray.3 = alias ptr, getelementptr inbounds ([4 x ptr], ptr getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, inrange i32 0), i32 0, i32 0) +; CHECK: @nestedarray.3 = alias ptr, getelementptr inbounds inrange(0, 4) ([4 x ptr], ptr @nestedarray, i32 0, i32 0) +@nestedarray.3 = alias ptr, getelementptr inbounds inrange(0, 4) ([4 x ptr], ptr getelementptr inbounds ([2 x [4 x ptr]], ptr @nestedarray, i32 0, i32 0), i32 0, i32 0) nhaehnle wrote: I never realized that we sometimes insert additional GEP expressions like that -- why? Is this just a left-over from typed pointer thinking that you intend to remove at some point? https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
@@ -47,28 +47,66 @@ static bool splitGlobal(GlobalVariable &GV) {
if (!Init)
return false;
- // Verify that each user of the global is an inrange getelementptr constant.
- // From this it follows that any loads from or stores to that global must use
- // a pointer derived from an inrange getelementptr constant, which is
- // sufficient to allow us to apply the splitting transform.
+ const DataLayout &DL = GV.getParent()->getDataLayout();
+ const StructLayout *SL = DL.getStructLayout(Init->getType());
+ ArrayRef MemberOffsets = SL->getMemberOffsets();
+ unsigned IndexWidth = DL.getIndexTypeSizeInBits(GV.getType());
+
+ // Verify that each user of the global is an inrange getelementptr constant,
+ // and collect information on how it relates to the global.
+ struct GEPInfo {
+GEPOperator *GEP;
+unsigned MemberIndex;
+APInt MemberRelativeOffset;
+
+GEPInfo(GEPOperator *GEP, unsigned MemberIndex, APInt MemberRelativeOffset)
+: GEP(GEP), MemberIndex(MemberIndex),
+ MemberRelativeOffset(std::move(MemberRelativeOffset)) {}
+ };
+ SmallVector Infos;
for (User *U : GV.users()) {
-if (!isa(U))
+auto *GEP = dyn_cast(U);
+if (!GEP)
return false;
-auto *GEP = dyn_cast(U);
-if (!GEP || !GEP->getInRangeIndex() || *GEP->getInRangeIndex() != 1 ||
-!isa(GEP->getOperand(1)) ||
-!cast(GEP->getOperand(1))->isZero() ||
-!isa(GEP->getOperand(2)))
+std::optional InRange = GEP->getInRange();
+if (!InRange)
+ return false;
+
+APInt Offset(IndexWidth, 0);
+if (!GEP->accumulateConstantOffset(DL, Offset))
+ return false;
+
+// Determine source-relative inrange.
+ConstantRange SrcInRange = InRange->sextOrTrunc(IndexWidth).add(Offset);
+
+// Check that the GEP offset is in the range (treating upper bound as
+// inclusive here).
+if (!SrcInRange.contains(Offset) && SrcInRange.getUpper() != Offset)
+ return false;
+
+// Find which struct member the range corresponds to.
+if (SrcInRange.getLower().uge(SL->getSizeInBytes()))
return false;
+
+unsigned MemberIndex =
+SL->getElementContainingOffset(SrcInRange.getLower().getZExtValue());
nhaehnle wrote:
Does this also need a check that SrcInRange.getLower() is non-negative?
https://github.com/llvm/llvm-project/pull/84341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
https://github.com/nhaehnle commented: The clang-format complaints look real. (I'm no friend of the alignment of end-of-line comments because it's super noisy in diffs, but...) I don't know Clang, but the changes there look reasonable enough to me. One nit inline, with that fixed the change LGTM. https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
https://github.com/nhaehnle edited https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
llvmbot wrote:
@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-lld-elf
Author: Nikita Popov (nikic)
Changes
As part of the [migration to
ptradd](https://discourse.llvm.org/t/rfc-replacing-getelementptr-with-ptradd/68699),
we need to change the representation of the `inrange` attribute, which is used
for vtable splitting.
Currently, inrange is specified as follows:
```
getelementptr inbounds ({ [4 x ptr], [4 x ptr] }, ptr @vt, i64 0,
inrange i32 1, i64 2)
```
The `inrange` is placed on a GEP index, and all accesses must be "in range" of
that index. The proposed new representation is as follows:
```
getelementptr inbounds inrange(-16, 16) ({ [4 x ptr], [4 x ptr] }, ptr @vt, i64 0, i32 1, i64 2)
```
This specifies which offsets are "in range" of the GEP result. The new
representation will continue working when canonicalizing to ptradd
representation:
```
getelementptr inbounds inrange(-16, 16) (i8, ptr @vt, i64 48)
```
The inrange offsets are relative to the return value of the GEP. An alternative
design could make them relative to the source pointer instead. The
result-relative format was chosen on the off-chance that we want to extend
support to non-constant GEPs in the future, in which case this variant is more
expressive.
This implementation "upgrades" the old inrange representation in bitcode by
simply dropping it. This is a very niche feature, and I don't think trying to
upgrade it is worthwhile. Let me know if you disagree.
---
Patch is 159.96 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/84341.diff
70 Files Affected:
- (modified) clang/lib/CodeGen/CGVTT.cpp (+11-2)
- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+14-6)
- (modified)
clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/auto-var-init.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/const-init-cxx11.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/constructor-init.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/copy-constructor-synthesis.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/dynamic-cast-exact.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/microsoft-interface.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
(+7-7)
- (modified) clang/test/CodeGenCXX/static-init.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/strict-vtable-pointers.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/visibility.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/vtable-assume-load-address-space.cpp (+7-7)
- (modified) clang/test/CodeGenCXX/vtable-assume-load.cpp (+7-7)
- (modified)
clang/test/CodeGenCXX/vtable-pointer-initialization-address-space.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtable-pointer-initialization.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtt-address-space.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/vtt-layout-address-space.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtt-layout.cpp (+4-4)
- (modified)
clang/test/OpenMP/target_data_use_device_ptr_inheritance_codegen.cpp (+6-6)
- (modified) lld/test/ELF/lto/Inputs/devirt_validate_vtable_typeinfos_ref.ll
(+2-2)
- (modified) lld/test/ELF/lto/Inputs/devirt_validate_vtable_typeinfos_undef.ll
(+1-1)
- (modified) lld/test/ELF/lto/devirt_split_unit_localize.ll (+1-1)
- (modified) lld/test/ELF/lto/devirt_validate_vtable_typeinfos_ref.ll (+1-1)
- (modified) lld/test/MachO/thinlto-split-unit-start-lib.ll (+2-2)
- (modified) llvm/docs/LangRef.rst (+10-10)
- (modified) llvm/include/llvm/AsmParser/LLParser.h (+1-2)
- (modified) llvm/include/llvm/Bitcode/LLVMBitCodes.h (+2-1)
- (modified) llvm/include/llvm/IR/ConstantFold.h (+1-1)
- (modified) llvm/include/llvm/IR/Constants.h (+7-6)
- (modified) llvm/include/llvm/IR/Operator.h (+1-6)
- (modified) llvm/lib/Analysis/ConstantFolding.cpp (+19-24)
- (modified) llvm/lib/AsmParser/LLParser.cpp (+40-17)
- (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+38-21)
- (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+4-3)
- (modified) llvm/lib/IR/AsmWriter.cpp (+4-6)
- (modified) llvm/lib/IR/ConstantFold.cpp (+13-22)
- (modified) llvm/lib/IR/Constants.cpp (+16-12)
- (modified) llvm/lib/IR/ConstantsContext.h (+38-13)
- (modified) llvm/lib/IR/Operator.cpp (+7-1)
- (modified) llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/GlobalSplit.cpp (+57-27)
- (modified) llvm/lib/Transforms/Utils/FunctionComparator.cpp (+13-3)
- (modified) llvm/test/Assembler/getelementptr.ll (+13-11)
- (added) llvm/test/Assembler/inrange-errors.ll (+46)
- (modified) llvm/test/Bitcode/compatibility-4.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility-5.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility-6.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility.ll (+2-2)
- (modified) llv
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
llvmbot wrote:
@llvm/pr-subscribers-lld-macho
Author: Nikita Popov (nikic)
Changes
As part of the [migration to
ptradd](https://discourse.llvm.org/t/rfc-replacing-getelementptr-with-ptradd/68699),
we need to change the representation of the `inrange` attribute, which is used
for vtable splitting.
Currently, inrange is specified as follows:
```
getelementptr inbounds ({ [4 x ptr], [4 x ptr] }, ptr @vt, i64 0,
inrange i32 1, i64 2)
```
The `inrange` is placed on a GEP index, and all accesses must be "in range" of
that index. The proposed new representation is as follows:
```
getelementptr inbounds inrange(-16, 16) ({ [4 x ptr], [4 x ptr] }, ptr @vt, i64 0, i32 1, i64 2)
```
This specifies which offsets are "in range" of the GEP result. The new
representation will continue working when canonicalizing to ptradd
representation:
```
getelementptr inbounds inrange(-16, 16) (i8, ptr @vt, i64 48)
```
The inrange offsets are relative to the return value of the GEP. An alternative
design could make them relative to the source pointer instead. The
result-relative format was chosen on the off-chance that we want to extend
support to non-constant GEPs in the future, in which case this variant is more
expressive.
This implementation "upgrades" the old inrange representation in bitcode by
simply dropping it. This is a very niche feature, and I don't think trying to
upgrade it is worthwhile. Let me know if you disagree.
---
Patch is 159.96 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/84341.diff
70 Files Affected:
- (modified) clang/lib/CodeGen/CGVTT.cpp (+11-2)
- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+14-6)
- (modified)
clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/auto-var-init.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/const-init-cxx11.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/constructor-init.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/copy-constructor-synthesis.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/dynamic-cast-exact.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/microsoft-interface.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
(+7-7)
- (modified) clang/test/CodeGenCXX/static-init.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/strict-vtable-pointers.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/visibility.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/vtable-assume-load-address-space.cpp (+7-7)
- (modified) clang/test/CodeGenCXX/vtable-assume-load.cpp (+7-7)
- (modified)
clang/test/CodeGenCXX/vtable-pointer-initialization-address-space.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtable-pointer-initialization.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtt-address-space.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/vtt-layout-address-space.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtt-layout.cpp (+4-4)
- (modified)
clang/test/OpenMP/target_data_use_device_ptr_inheritance_codegen.cpp (+6-6)
- (modified) lld/test/ELF/lto/Inputs/devirt_validate_vtable_typeinfos_ref.ll
(+2-2)
- (modified) lld/test/ELF/lto/Inputs/devirt_validate_vtable_typeinfos_undef.ll
(+1-1)
- (modified) lld/test/ELF/lto/devirt_split_unit_localize.ll (+1-1)
- (modified) lld/test/ELF/lto/devirt_validate_vtable_typeinfos_ref.ll (+1-1)
- (modified) lld/test/MachO/thinlto-split-unit-start-lib.ll (+2-2)
- (modified) llvm/docs/LangRef.rst (+10-10)
- (modified) llvm/include/llvm/AsmParser/LLParser.h (+1-2)
- (modified) llvm/include/llvm/Bitcode/LLVMBitCodes.h (+2-1)
- (modified) llvm/include/llvm/IR/ConstantFold.h (+1-1)
- (modified) llvm/include/llvm/IR/Constants.h (+7-6)
- (modified) llvm/include/llvm/IR/Operator.h (+1-6)
- (modified) llvm/lib/Analysis/ConstantFolding.cpp (+19-24)
- (modified) llvm/lib/AsmParser/LLParser.cpp (+40-17)
- (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+38-21)
- (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+4-3)
- (modified) llvm/lib/IR/AsmWriter.cpp (+4-6)
- (modified) llvm/lib/IR/ConstantFold.cpp (+13-22)
- (modified) llvm/lib/IR/Constants.cpp (+16-12)
- (modified) llvm/lib/IR/ConstantsContext.h (+38-13)
- (modified) llvm/lib/IR/Operator.cpp (+7-1)
- (modified) llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/GlobalSplit.cpp (+57-27)
- (modified) llvm/lib/Transforms/Utils/FunctionComparator.cpp (+13-3)
- (modified) llvm/test/Assembler/getelementptr.ll (+13-11)
- (added) llvm/test/Assembler/inrange-errors.ll (+46)
- (modified) llvm/test/Bitcode/compatibility-4.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility-5.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility-6.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility.ll (+2-2)
- (modified) llvm/test/Other/optimize-inrange-gep.ll (+14-4)
- (modified) l
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: Nikita Popov (nikic)
Changes
As part of the [migration to
ptradd](https://discourse.llvm.org/t/rfc-replacing-getelementptr-with-ptradd/68699),
we need to change the representation of the `inrange` attribute, which is used
for vtable splitting.
Currently, inrange is specified as follows:
```
getelementptr inbounds ({ [4 x ptr], [4 x ptr] }, ptr @vt, i64 0,
inrange i32 1, i64 2)
```
The `inrange` is placed on a GEP index, and all accesses must be "in range" of
that index. The proposed new representation is as follows:
```
getelementptr inbounds inrange(-16, 16) ({ [4 x ptr], [4 x ptr] }, ptr @vt, i64 0, i32 1, i64 2)
```
This specifies which offsets are "in range" of the GEP result. The new
representation will continue working when canonicalizing to ptradd
representation:
```
getelementptr inbounds inrange(-16, 16) (i8, ptr @vt, i64 48)
```
The inrange offsets are relative to the return value of the GEP. An alternative
design could make them relative to the source pointer instead. The
result-relative format was chosen on the off-chance that we want to extend
support to non-constant GEPs in the future, in which case this variant is more
expressive.
This implementation "upgrades" the old inrange representation in bitcode by
simply dropping it. This is a very niche feature, and I don't think trying to
upgrade it is worthwhile. Let me know if you disagree.
---
Patch is 159.96 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/84341.diff
70 Files Affected:
- (modified) clang/lib/CodeGen/CGVTT.cpp (+11-2)
- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+14-6)
- (modified)
clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/auto-var-init.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/const-init-cxx11.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/constructor-init.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/copy-constructor-synthesis.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/dynamic-cast-exact.cpp (+3-3)
- (modified) clang/test/CodeGenCXX/microsoft-interface.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
(+7-7)
- (modified) clang/test/CodeGenCXX/static-init.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/strict-vtable-pointers.cpp (+2-2)
- (modified) clang/test/CodeGenCXX/visibility.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/vtable-assume-load-address-space.cpp (+7-7)
- (modified) clang/test/CodeGenCXX/vtable-assume-load.cpp (+7-7)
- (modified)
clang/test/CodeGenCXX/vtable-pointer-initialization-address-space.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtable-pointer-initialization.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtt-address-space.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/vtt-layout-address-space.cpp (+4-4)
- (modified) clang/test/CodeGenCXX/vtt-layout.cpp (+4-4)
- (modified)
clang/test/OpenMP/target_data_use_device_ptr_inheritance_codegen.cpp (+6-6)
- (modified) lld/test/ELF/lto/Inputs/devirt_validate_vtable_typeinfos_ref.ll
(+2-2)
- (modified) lld/test/ELF/lto/Inputs/devirt_validate_vtable_typeinfos_undef.ll
(+1-1)
- (modified) lld/test/ELF/lto/devirt_split_unit_localize.ll (+1-1)
- (modified) lld/test/ELF/lto/devirt_validate_vtable_typeinfos_ref.ll (+1-1)
- (modified) lld/test/MachO/thinlto-split-unit-start-lib.ll (+2-2)
- (modified) llvm/docs/LangRef.rst (+10-10)
- (modified) llvm/include/llvm/AsmParser/LLParser.h (+1-2)
- (modified) llvm/include/llvm/Bitcode/LLVMBitCodes.h (+2-1)
- (modified) llvm/include/llvm/IR/ConstantFold.h (+1-1)
- (modified) llvm/include/llvm/IR/Constants.h (+7-6)
- (modified) llvm/include/llvm/IR/Operator.h (+1-6)
- (modified) llvm/lib/Analysis/ConstantFolding.cpp (+19-24)
- (modified) llvm/lib/AsmParser/LLParser.cpp (+40-17)
- (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+38-21)
- (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+4-3)
- (modified) llvm/lib/IR/AsmWriter.cpp (+4-6)
- (modified) llvm/lib/IR/ConstantFold.cpp (+13-22)
- (modified) llvm/lib/IR/Constants.cpp (+16-12)
- (modified) llvm/lib/IR/ConstantsContext.h (+38-13)
- (modified) llvm/lib/IR/Operator.cpp (+7-1)
- (modified) llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/GlobalSplit.cpp (+57-27)
- (modified) llvm/lib/Transforms/Utils/FunctionComparator.cpp (+13-3)
- (modified) llvm/test/Assembler/getelementptr.ll (+13-11)
- (added) llvm/test/Assembler/inrange-errors.ll (+46)
- (modified) llvm/test/Bitcode/compatibility-4.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility-5.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility-6.0.ll (+1-1)
- (modified) llvm/test/Bitcode/compatibility.ll (+2-2)
- (modified) llvm/test/Other/optimize-inrange-gep.ll (+14-4)
- (modifie
[clang] [lld] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
https://github.com/nikic ready_for_review https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
