Skip to content

Commit 847c83f

Browse files
arsenmarsenm
authored
InstCombine: Process addrspacecast uses in PointerReplacer (llvm#91953)
This was looking through an addrspacecast, and not finding a later unfoldable cast to another address space. Fixes improperly deleting a required alloca + memcpy and introducing an illegal addrspacecast. This also required fixing some worklist management issues with addrspacecast, and assuming that only memcpy sources could need replacement. Regresses one test function, but this looks like it optimized before by accident. It never saw the pointer use by the call to readonly_callee, which should require insertion of a new cast. Fixes llvm#68120
1 parent 74d91d9 commit 847c83f

File tree

3 files changed

+104
-16
lines changed

3 files changed

+104
-16
lines changed

‎llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,13 @@ bool PointerReplacer::collectUsersRecursive(Instruction &I) {
342342
Worklist.insert(Inst);
343343
} else if (isEqualOrValidAddrSpaceCast(Inst, FromAS)) {
344344
Worklist.insert(Inst);
345+
if (!collectUsersRecursive(*Inst))
346+
return false;
345347
} else if (Inst->isLifetimeStartOrEnd()) {
346348
continue;
347349
} else {
350+
// TODO: For arbitrary uses with address space mismatches, should we check
351+
// if we can introduce a valid addrspacecast?
348352
LLVM_DEBUG(dbgs() << "Cannot handle pointer user: " << *U << '\n');
349353
return false;
350354
}
@@ -406,20 +410,18 @@ void PointerReplacer::replace(Instruction *I) {
406410
NewSI->takeName(SI);
407411
WorkMap[SI] = NewSI;
408412
} else if (auto *MemCpy = dyn_cast<MemTransferInst>(I)) {
409-
auto *SrcV = getReplacement(MemCpy->getRawSource());
410-
// The pointer may appear in the destination of a copy, but we don't want to
411-
// replace it.
412-
if (!SrcV) {
413-
assert(getReplacement(MemCpy->getRawDest()) &&
414-
"destination not in replace list");
415-
return;
416-
}
413+
auto *DestV = MemCpy->getRawDest();
414+
auto *SrcV = MemCpy->getRawSource();
415+
416+
if (auto *DestReplace = getReplacement(DestV))
417+
DestV = DestReplace;
418+
if (auto *SrcReplace = getReplacement(SrcV))
419+
SrcV = SrcReplace;
417420

418421
IC.Builder.SetInsertPoint(MemCpy);
419422
auto *NewI = IC.Builder.CreateMemTransferInst(
420-
MemCpy->getIntrinsicID(), MemCpy->getRawDest(), MemCpy->getDestAlign(),
421-
SrcV, MemCpy->getSourceAlign(), MemCpy->getLength(),
422-
MemCpy->isVolatile());
423+
MemCpy->getIntrinsicID(), DestV, MemCpy->getDestAlign(), SrcV,
424+
MemCpy->getSourceAlign(), MemCpy->getLength(), MemCpy->isVolatile());
423425
AAMDNodes AAMD = MemCpy->getAAMetadata();
424426
if (AAMD)
425427
NewI->setAAMetadata(AAMD);
@@ -432,16 +434,17 @@ void PointerReplacer::replace(Instruction *I) {
432434
assert(isEqualOrValidAddrSpaceCast(
433435
ASC, V->getType()->getPointerAddressSpace()) &&
434436
"Invalid address space cast!");
435-
auto *NewV = V;
437+
436438
if (V->getType()->getPointerAddressSpace() !=
437439
ASC->getType()->getPointerAddressSpace()) {
438440
auto *NewI = new AddrSpaceCastInst(V, ASC->getType(), "");
439441
NewI->takeName(ASC);
440442
IC.InsertNewInstWith(NewI, ASC->getIterator());
441-
NewV = NewI;
443+
WorkMap[ASC] = NewI;
444+
} else {
445+
WorkMap[ASC] = V;
442446
}
443-
IC.replaceInstUsesWith(*ASC, NewV);
444-
IC.eraseInstFromFunction(*ASC);
447+
445448
} else {
446449
llvm_unreachable("should never reach here");
447450
}

‎llvm/test/Transforms/InstCombine/AMDGPU/issue68120.ll

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
2+
; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -passes=instcombine %s | FileCheck %s
3+
4+
; It is illegal to pass ptr addrspace(4) in %arg by addrspacecasting
5+
; to ptr addrspace(5) to pass off to the stack argument. A temporary
6+
; alloca and memcpy is necessary.
7+
define void @issue68120_invalid_addrspacecast_introduced_0(ptr addrspace(4) byref([56 x i8]) %arg) {
8+
; CHECK-LABEL: define void @issue68120_invalid_addrspacecast_introduced_0(
9+
; CHECK-SAME: ptr addrspace(4) byref([56 x i8]) [[ARG:%.*]]) {
10+
; CHECK-NEXT: [[ADDRSPACECAST_0_TO_5:%.*]] = alloca [56 x i8], align 1, addrspace(5)
11+
; CHECK-NEXT: call void @llvm.memcpy.p5.p4.i64(ptr addrspace(5) noundef align 1 dereferenceable(56) [[ADDRSPACECAST_0_TO_5]], ptr addrspace(4) noundef align 1 dereferenceable(56) [[ARG]], i64 56, i1 false)
12+
; CHECK-NEXT: call void @byval_func(ptr addrspace(5) [[ADDRSPACECAST_0_TO_5]])
13+
; CHECK-NEXT: ret void
14+
;
15+
%alloca = alloca [56 x i8], addrspace(5)
16+
%alloca1 = alloca [56 x i8], addrspace(5)
17+
call void @llvm.memcpy.p5.p4.i64(ptr addrspace(5) %alloca, ptr addrspace(4) %arg, i64 56, i1 false)
18+
call void @llvm.memcpy.p5.p5.i64(ptr addrspace(5) %alloca1, ptr addrspace(5) %alloca, i64 56, i1 false)
19+
%addrspacecast.alloca1 = addrspacecast ptr addrspace(5) %alloca1 to ptr
20+
%addrspacecast.0.to.5 = addrspacecast ptr %addrspacecast.alloca1 to ptr addrspace(5)
21+
call void @byval_func(ptr addrspace(5) %addrspacecast.0.to.5)
22+
ret void
23+
}
24+
25+
; Further reduced variant that already eliminated one of the copies
26+
define void @issue68120_invalid_addrspacecast_introduced_1(ptr addrspace(4) byref([56 x i8]) %arg) {
27+
; CHECK-LABEL: define void @issue68120_invalid_addrspacecast_introduced_1(
28+
; CHECK-SAME: ptr addrspace(4) byref([56 x i8]) [[ARG:%.*]]) {
29+
; CHECK-NEXT: [[ADDRSPACECAST_0_TO_5:%.*]] = alloca [56 x i8], align 1, addrspace(5)
30+
; CHECK-NEXT: call void @llvm.memcpy.p5.p4.i64(ptr addrspace(5) noundef align 1 dereferenceable(56) [[ADDRSPACECAST_0_TO_5]], ptr addrspace(4) noundef align 1 dereferenceable(56) [[ARG]], i64 56, i1 false)
31+
; CHECK-NEXT: call void @byval_func(ptr addrspace(5) [[ADDRSPACECAST_0_TO_5]])
32+
; CHECK-NEXT: ret void
33+
;
34+
%alloca = alloca [56 x i8], addrspace(5)
35+
call void @llvm.memcpy.p5.p4.i64(ptr addrspace(5) %alloca, ptr addrspace(4) %arg, i64 56, i1 false)
36+
%addrspacecast.alloca = addrspacecast ptr addrspace(5) %alloca to ptr
37+
%addrspacecast.0.to.5 = addrspacecast ptr %addrspacecast.alloca to ptr addrspace(5)
38+
call void @byval_func(ptr addrspace(5) %addrspacecast.0.to.5)
39+
ret void
40+
}
41+
42+
define void @issue68120_use_cast_to_as0(ptr addrspace(4) byref([56 x i8]) %arg) {
43+
; CHECK-LABEL: define void @issue68120_use_cast_to_as0(
44+
; CHECK-SAME: ptr addrspace(4) byref([56 x i8]) [[ARG:%.*]]) {
45+
; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [56 x i8], align 1, addrspace(5)
46+
; CHECK-NEXT: call void @llvm.memcpy.p5.p4.i64(ptr addrspace(5) noundef align 1 dereferenceable(56) [[ALLOCA]], ptr addrspace(4) noundef align 1 dereferenceable(56) [[ARG]], i64 56, i1 false)
47+
; CHECK-NEXT: [[ADDRSPACECAST_ALLOCA:%.*]] = addrspacecast ptr addrspace(5) [[ALLOCA]] to ptr
48+
; CHECK-NEXT: call void @other_func(ptr [[ADDRSPACECAST_ALLOCA]])
49+
; CHECK-NEXT: ret void
50+
;
51+
%alloca = alloca [56 x i8], addrspace(5)
52+
call void @llvm.memcpy.p5.p4.i64(ptr addrspace(5) %alloca, ptr addrspace(4) %arg, i64 56, i1 false)
53+
%addrspacecast.alloca = addrspacecast ptr addrspace(5) %alloca to ptr
54+
%addrspacecast.0.to.5 = addrspacecast ptr %addrspacecast.alloca to ptr addrspace(5)
55+
call void @other_func(ptr %addrspacecast.alloca)
56+
ret void
57+
}
58+
59+
define void @issue68120_uses_invalid_addrspacecast(ptr addrspace(4) byref([56 x i8]) %arg) {
60+
; CHECK-LABEL: define void @issue68120_uses_invalid_addrspacecast(
61+
; CHECK-SAME: ptr addrspace(4) byref([56 x i8]) [[ARG:%.*]]) {
62+
; CHECK-NEXT: [[ADDRSPACECAST_0_TO_5:%.*]] = alloca [56 x i8], align 1, addrspace(5)
63+
; CHECK-NEXT: call void @llvm.memcpy.p5.p4.i64(ptr addrspace(5) noundef align 1 dereferenceable(56) [[ADDRSPACECAST_0_TO_5]], ptr addrspace(4) noundef align 1 dereferenceable(56) [[ARG]], i64 56, i1 false)
64+
; CHECK-NEXT: call void @byval_func(ptr addrspace(5) [[ADDRSPACECAST_0_TO_5]])
65+
; CHECK-NEXT: ret void
66+
;
67+
%alloca = alloca [56 x i8], addrspace(5)
68+
%alloca1 = alloca [56 x i8], addrspace(5)
69+
%illegal.cast = addrspacecast ptr addrspace(4) %arg to ptr addrspace(5)
70+
call void @llvm.memcpy.p5.p5.i64(ptr addrspace(5) %alloca, ptr addrspace(5) %illegal.cast, i64 56, i1 false)
71+
call void @llvm.memcpy.p5.p5.i64(ptr addrspace(5) %alloca1, ptr addrspace(5) %alloca, i64 56, i1 false)
72+
%addrspacecast.alloca1 = addrspacecast ptr addrspace(5) %alloca1 to ptr
73+
%addrspacecast.0.to.5 = addrspacecast ptr %addrspacecast.alloca1 to ptr addrspace(5)
74+
call void @byval_func(ptr addrspace(5) %addrspacecast.0.to.5)
75+
ret void
76+
}
77+
78+
79+
declare void @byval_func(ptr addrspace(5) byval([56 x i8]))
80+
declare void @other_func(ptr)
81+

‎llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,13 @@ entry:
429429

430430
declare i8 @readonly_callee(ptr readonly nocapture)
431431

432+
; FIXME: This should be able to fold to call i8 @readonly_callee(ptr nonnull @g1)
432433
define i8 @call_readonly_remove_alloca() {
433434
; CHECK-LABEL: @call_readonly_remove_alloca(
434-
; CHECK-NEXT: [[V:%.*]] = call i8 @readonly_callee(ptr nonnull @g1)
435+
; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [32 x i8], align 1, addrspace(1)
436+
; CHECK-NEXT: call void @llvm.memcpy.p1.p0.i64(ptr addrspace(1) noundef align 1 dereferenceable(32) [[ALLOCA]], ptr noundef nonnull align 16 dereferenceable(32) @g1, i64 32, i1 false)
437+
; CHECK-NEXT: [[P:%.*]] = addrspacecast ptr addrspace(1) [[ALLOCA]] to ptr
438+
; CHECK-NEXT: [[V:%.*]] = call i8 @readonly_callee(ptr [[P]])
435439
; CHECK-NEXT: ret i8 [[V]]
436440
;
437441
%alloca = alloca [32 x i8], addrspace(1)

0 commit comments

Comments
 (0)