RFC: [TTI] Assume casts between aliasing addrspaces are valid. NFCI. #137969
Conversation
Change the default implementation of isValidAddrSpaceCast to assume that casts between any address spaces satisfying addrspacesMayAlias are valid. This seems like a reasonable default assumption and targets can still override it if they need something different. This simplifies away the AMDGPU implementation.
|
@llvm/pr-subscribers-llvm-analysis Author: Jay Foad (jayfoad) ChangesChange the default implementation of isValidAddrSpaceCast to assume that This simplifies away the AMDGPU implementation. Full diff: https://github.com/llvm/llvm-project/pull/137969.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
index cd0e8769f5eb6..93ff091d4599a 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -136,7 +136,7 @@ class TargetTransformInfoImplBase {
virtual bool isAlwaysUniform(const Value *V) const { return false; }
virtual bool isValidAddrSpaceCast(unsigned FromAS, unsigned ToAS) const {
- return false;
+ return addrspacesMayAlias(FromAS, ToAS);
}
virtual bool addrspacesMayAlias(unsigned AS0, unsigned AS1) const {
diff --git a/llvm/include/llvm/CodeGen/BasicTTIImpl.h b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
index 6e2f65c01bf77..5cb8136aeb624 100644
--- a/llvm/include/llvm/CodeGen/BasicTTIImpl.h
+++ b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
@@ -387,10 +387,6 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
bool isAlwaysUniform(const Value *V) const override { return false; }
- bool isValidAddrSpaceCast(unsigned FromAS, unsigned ToAS) const override {
- return false;
- }
-
bool addrspacesMayAlias(unsigned AS0, unsigned AS1) const override {
return true;
}
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
index f6f7bd4bfcf5b..9b0adcaa19aea 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
@@ -177,27 +177,6 @@ class GCNTTIImpl final : public BasicTTIImplBase<GCNTTIImpl> {
bool isSourceOfDivergence(const Value *V) const override;
bool isAlwaysUniform(const Value *V) const override;
- bool isValidAddrSpaceCast(unsigned FromAS, unsigned ToAS) const override {
- // Address space casts must cast between different address spaces.
- if (FromAS == ToAS)
- return false;
-
- if (FromAS == AMDGPUAS::FLAT_ADDRESS)
- return AMDGPU::isExtendedGlobalAddrSpace(ToAS) ||
- ToAS == AMDGPUAS::LOCAL_ADDRESS ||
- ToAS == AMDGPUAS::PRIVATE_ADDRESS;
-
- if (AMDGPU::isExtendedGlobalAddrSpace(FromAS))
- return AMDGPU::isFlatGlobalAddrSpace(ToAS) ||
- ToAS == AMDGPUAS::CONSTANT_ADDRESS_32BIT;
-
- if (FromAS == AMDGPUAS::LOCAL_ADDRESS ||
- FromAS == AMDGPUAS::PRIVATE_ADDRESS)
- return ToAS == AMDGPUAS::FLAT_ADDRESS;
-
- return false;
- }
-
bool addrspacesMayAlias(unsigned AS0, unsigned AS1) const override {
return AMDGPU::addrspacesMayAlias(AS0, AS1);
}
|
|
@llvm/pr-subscribers-backend-amdgpu Author: Jay Foad (jayfoad) ChangesChange the default implementation of isValidAddrSpaceCast to assume that This simplifies away the AMDGPU implementation. Full diff: https://github.com/llvm/llvm-project/pull/137969.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
index cd0e8769f5eb6..93ff091d4599a 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -136,7 +136,7 @@ class TargetTransformInfoImplBase {
virtual bool isAlwaysUniform(const Value *V) const { return false; }
virtual bool isValidAddrSpaceCast(unsigned FromAS, unsigned ToAS) const {
- return false;
+ return addrspacesMayAlias(FromAS, ToAS);
}
virtual bool addrspacesMayAlias(unsigned AS0, unsigned AS1) const {
diff --git a/llvm/include/llvm/CodeGen/BasicTTIImpl.h b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
index 6e2f65c01bf77..5cb8136aeb624 100644
--- a/llvm/include/llvm/CodeGen/BasicTTIImpl.h
+++ b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
@@ -387,10 +387,6 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
bool isAlwaysUniform(const Value *V) const override { return false; }
- bool isValidAddrSpaceCast(unsigned FromAS, unsigned ToAS) const override {
- return false;
- }
-
bool addrspacesMayAlias(unsigned AS0, unsigned AS1) const override {
return true;
}
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
index f6f7bd4bfcf5b..9b0adcaa19aea 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
@@ -177,27 +177,6 @@ class GCNTTIImpl final : public BasicTTIImplBase<GCNTTIImpl> {
bool isSourceOfDivergence(const Value *V) const override;
bool isAlwaysUniform(const Value *V) const override;
- bool isValidAddrSpaceCast(unsigned FromAS, unsigned ToAS) const override {
- // Address space casts must cast between different address spaces.
- if (FromAS == ToAS)
- return false;
-
- if (FromAS == AMDGPUAS::FLAT_ADDRESS)
- return AMDGPU::isExtendedGlobalAddrSpace(ToAS) ||
- ToAS == AMDGPUAS::LOCAL_ADDRESS ||
- ToAS == AMDGPUAS::PRIVATE_ADDRESS;
-
- if (AMDGPU::isExtendedGlobalAddrSpace(FromAS))
- return AMDGPU::isFlatGlobalAddrSpace(ToAS) ||
- ToAS == AMDGPUAS::CONSTANT_ADDRESS_32BIT;
-
- if (FromAS == AMDGPUAS::LOCAL_ADDRESS ||
- FromAS == AMDGPUAS::PRIVATE_ADDRESS)
- return ToAS == AMDGPUAS::FLAT_ADDRESS;
-
- return false;
- }
-
bool addrspacesMayAlias(unsigned AS0, unsigned AS1) const override {
return AMDGPU::addrspacesMayAlias(AS0, AS1);
}
|
| @@ -387,10 +387,6 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> { | |||
|
|
|||
| bool isAlwaysUniform(const Value *V) const override { return false; } | |||
|
|
|||
| bool isValidAddrSpaceCast(unsigned FromAS, unsigned ToAS) const override { | |||
There was a problem hiding this comment.
TBH I don't understand the relationship between BasicTTIImpl and TargetTransformInfoImplBase. I just removed this to stop it from overriding my default implementation in TTIIB.
There was a problem hiding this comment.
TTIImplBase has two derived classes, BasicTTIImpl and NoTTIImpl. The latter is used when the target doesn't provide its own implementation of TTIImplBase (which is usually, if not always, done by inheriting from BasicTTIImpl).
There was a problem hiding this comment.
Probably anything that doesn't make use of TM/TLI (like this and surrounding methods) should only be in TTIImplBase rather than BasicTTIImpl?
There was a problem hiding this comment.
I don't think this is the right thing to do, as these should have inverse defaults. We want to assume that all address spaces alias and no addrspacecasts are valid by default. A precise addrspacesMayAlias + isValidAddrSpaceCast will converge to being the same, but if either of them are approximate (including the fully conservative default approximation) they will differ.
Fair enough, I can handle this in the target instead: #137986. |
Change the default implementation of isValidAddrSpaceCast to assume that
casts between any address spaces satisfying addrspacesMayAlias are
valid. This seems like a reasonable default assumption and targets can
still override it if they need something different.
This simplifies away the AMDGPU implementation.