-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[InferAddressSpaces] Initialize op(generic const, generic const, ...) -> generic #172143
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
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Hongyu Chen (XChy) ChangesFixes #171890 Full diff: https://github.com/llvm/llvm-project/pull/172143.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
index 352a1b331001a..d7caa60055750 100644
--- a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -1048,7 +1048,8 @@ bool InferAddressSpacesImpl::updateAddressSpace(
} else {
// Otherwise, infer the address space from its pointer operands.
SmallVector<Constant *, 2> ConstantPtrOps;
- for (Value *PtrOperand : getPointerOperands(V, *DL, TTI)) {
+ auto PtrOps = getPointerOperands(V, *DL, TTI);
+ for (Value *PtrOperand : PtrOps) {
auto I = InferredAddrSpace.find(PtrOperand);
unsigned OperandAS;
if (I == InferredAddrSpace.end()) {
@@ -1079,12 +1080,18 @@ bool InferAddressSpacesImpl::updateAddressSpace(
if (NewAS == FlatAddrSpace)
break;
}
+
if (NewAS != FlatAddrSpace && NewAS != UninitializedAddressSpace) {
if (any_of(ConstantPtrOps, [=](Constant *C) {
return !isSafeToCastConstAddrSpace(C, NewAS);
}))
NewAS = FlatAddrSpace;
}
+
+ // operator(uninit const, uninit const, ...) -> flat
+ if (NewAS == UninitializedAddressSpace &&
+ PtrOps.size() == ConstantPtrOps.size())
+ NewAS = FlatAddrSpace;
}
unsigned OldAS = InferredAddrSpace.lookup(&V);
diff --git a/llvm/test/Transforms/InferAddressSpaces/NVPTX/nullptr.ll b/llvm/test/Transforms/InferAddressSpaces/NVPTX/nullptr.ll
new file mode 100644
index 0000000000000..516d90d282f78
--- /dev/null
+++ b/llvm/test/Transforms/InferAddressSpaces/NVPTX/nullptr.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=infer-address-spaces %s | FileCheck %s
+
+target triple = "nvptx64-unknown-nvidiacl"
+
+define ptr @pr171890() {
+; CHECK-LABEL: define ptr @pr171890() {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[STACK:%.*]] = alloca i16, align 2
+; CHECK-NEXT: [[TMP0:%.*]] = addrspacecast ptr [[STACK]] to ptr addrspace(5)
+; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr addrspace(5) [[TMP0]] to ptr
+; CHECK-NEXT: br label %[[IF:.*]]
+; CHECK: [[IF]]:
+; CHECK-NEXT: [[NULLPHI:%.*]] = phi ptr [ null, %[[ENTRY]] ]
+; CHECK-NEXT: br i1 false, label %[[THEN:.*]], label %[[ELSE:.*]]
+; CHECK: [[ELSE]]:
+; CHECK-NEXT: br label %[[THEN]]
+; CHECK: [[THEN]]:
+; CHECK-NEXT: [[PHI2:%.*]] = phi ptr [ [[TMP1]], %[[IF]] ], [ [[NULLPHI]], %[[ELSE]] ]
+; CHECK-NEXT: ret ptr [[PHI2]]
+;
+entry:
+ %stack = alloca i16, align 2
+ br label %if
+
+if:
+ %nullphi = phi ptr [ null, %entry ]
+ br i1 false, label %then, label %else
+
+else:
+ br label %then
+
+then:
+ %phi2 = phi ptr [ %stack, %if ], [ %nullphi, %else ]
+ ret ptr %phi2
+}
|
|
@llvm/pr-subscribers-backend-nvptx Author: Hongyu Chen (XChy) ChangesFixes #171890 Full diff: https://github.com/llvm/llvm-project/pull/172143.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
index 352a1b331001a..d7caa60055750 100644
--- a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -1048,7 +1048,8 @@ bool InferAddressSpacesImpl::updateAddressSpace(
} else {
// Otherwise, infer the address space from its pointer operands.
SmallVector<Constant *, 2> ConstantPtrOps;
- for (Value *PtrOperand : getPointerOperands(V, *DL, TTI)) {
+ auto PtrOps = getPointerOperands(V, *DL, TTI);
+ for (Value *PtrOperand : PtrOps) {
auto I = InferredAddrSpace.find(PtrOperand);
unsigned OperandAS;
if (I == InferredAddrSpace.end()) {
@@ -1079,12 +1080,18 @@ bool InferAddressSpacesImpl::updateAddressSpace(
if (NewAS == FlatAddrSpace)
break;
}
+
if (NewAS != FlatAddrSpace && NewAS != UninitializedAddressSpace) {
if (any_of(ConstantPtrOps, [=](Constant *C) {
return !isSafeToCastConstAddrSpace(C, NewAS);
}))
NewAS = FlatAddrSpace;
}
+
+ // operator(uninit const, uninit const, ...) -> flat
+ if (NewAS == UninitializedAddressSpace &&
+ PtrOps.size() == ConstantPtrOps.size())
+ NewAS = FlatAddrSpace;
}
unsigned OldAS = InferredAddrSpace.lookup(&V);
diff --git a/llvm/test/Transforms/InferAddressSpaces/NVPTX/nullptr.ll b/llvm/test/Transforms/InferAddressSpaces/NVPTX/nullptr.ll
new file mode 100644
index 0000000000000..516d90d282f78
--- /dev/null
+++ b/llvm/test/Transforms/InferAddressSpaces/NVPTX/nullptr.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=infer-address-spaces %s | FileCheck %s
+
+target triple = "nvptx64-unknown-nvidiacl"
+
+define ptr @pr171890() {
+; CHECK-LABEL: define ptr @pr171890() {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[STACK:%.*]] = alloca i16, align 2
+; CHECK-NEXT: [[TMP0:%.*]] = addrspacecast ptr [[STACK]] to ptr addrspace(5)
+; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr addrspace(5) [[TMP0]] to ptr
+; CHECK-NEXT: br label %[[IF:.*]]
+; CHECK: [[IF]]:
+; CHECK-NEXT: [[NULLPHI:%.*]] = phi ptr [ null, %[[ENTRY]] ]
+; CHECK-NEXT: br i1 false, label %[[THEN:.*]], label %[[ELSE:.*]]
+; CHECK: [[ELSE]]:
+; CHECK-NEXT: br label %[[THEN]]
+; CHECK: [[THEN]]:
+; CHECK-NEXT: [[PHI2:%.*]] = phi ptr [ [[TMP1]], %[[IF]] ], [ [[NULLPHI]], %[[ELSE]] ]
+; CHECK-NEXT: ret ptr [[PHI2]]
+;
+entry:
+ %stack = alloca i16, align 2
+ br label %if
+
+if:
+ %nullphi = phi ptr [ null, %entry ]
+ br i1 false, label %then, label %else
+
+else:
+ br label %then
+
+then:
+ %phi2 = phi ptr [ %stack, %if ], [ %nullphi, %else ]
+ ret ptr %phi2
+}
|
wenju-he
left a comment
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.
LGTM
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.
Not quite related to this PR, but there is still opportunity to infer AS from ConstantPtrOps if any of them's AS can be inferred, right?
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.
Yes, this code only has an impact when no AS in ConstantPtrOps is inferred.
arsenm
left a comment
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.
If the pointer operands of an instruction are all constants, we may infer the AS of the instruction as uninitialized finally.
I don't understand this logic. You can't infer anything about the specific address space from the fact that they are constant? Sure, you could do this if all of the constants were undef but not an arbitrary defined constant
Sorry, maybe the description mixes "uninitialized" and "generic". I think the more precise description of the problem is "If the pointer operands of an instruction are all constants with generic AS, we always infer the AS of the instruction as uninitialized finally." In In the testcase, the AS of |
an example would be |
sorry, please ignore. My above comment is for |
Yes. It's possible to infer the AS from these constant expressions, but the current implementation doesn't consider it. |
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.
no -o - here?
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.
Is it necessary? opt -S already writes to stdout.
Fixes #171890
If the pointer operands of an instruction are all constants with generic AS, we always infer the AS of the instruction as uninitialized finally. And the rewrite process will skip cloning the instruction, producing invalid IR.
This patch fixes it by inferring the AS of this kind of instruction as flat. Maybe we can fold the operator with all constants to get better performance, but I think this case is rare in the real world.