[PATCH] D120586: [Attributor] Add AAAddressSpaceInfo to deduce address spaces
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 25 14:30:11 PST 2022
jdoerfert added inline comments.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9910
+ return getAssumed();
+ }
+
----------------
hasXXX sounds like a boolean return, this would be getAS
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9917
+
+ A.getAAFor<AAIsDead>(*this, getIRPosition(), DepClassTy::REQUIRED);
+ }
----------------
don't make it required, make the dependence type NONE
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9924
+ AA::getAssumedUnderlyingObjects(A, getAssociatedValue(), Objects, *this,
+ getCtxI(), UsedAssumedInformation);
+
----------------
Check the return type of this function.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9931
+ if (!V || !V->getType()->isPointerTy())
+ continue;
+
----------------
That doesn't work. We cannot ignore things with the "wrong type".
V should never be null, undef can be ignored, a "zero" too if nullptr is not valid, everything else not I think.
See some other uses of getAssumedUnderlyingObjects.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9938
+ else if (AddressSpace != NewAddressSpace)
+ AddressSpace = getWorstState();
+ }
----------------
break
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9957
+ Instruction *Cast = new AddrSpaceCastInst(AssociatedValue, NewPtrTy);
+ Cast->insertAfter(CtxI);
+
----------------
before, not after.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9982
+ }
+ }
+
----------------
We should reuse the rewrite capabilities of the existing pass, or at least clone loads/stores properly so we don't loose metadata and such.
================
Comment at: llvm/test/Transforms/Attributor/IPConstantProp/2009-09-24-byval-ptr.ll:233
; IS__CGSCC_NPM-NEXT: [[TMP4:%.*]] = getelementptr [[STRUCT_MYSTR]], %struct.MYstr* [[U_PRIV]], i32 0, i32 0
-; IS__CGSCC_NPM-NEXT: [[TMP5:%.*]] = load i8, i8* [[TMP4]], align 8
+; IS__CGSCC_NPM-NEXT: [[TMP5:%.*]] = load i8, i8* undef, align 8
; IS__CGSCC_NPM-NEXT: [[TMP6:%.*]] = zext i8 0 to i32
----------------
This indicates a problem, though, not necessarily in your code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120586/new/
https://reviews.llvm.org/D120586
More information about the llvm-commits
mailing list