[PATCH] D120586: [Attributor] Add AAAddressSpaceInfo to deduce address spaces
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 6 15:14:15 PDT 2023
jdoerfert added inline comments.
================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:5981
+ : StateWrapper<BooleanState, AbstractAttribute>(IRP) {}
+
+ /// Return the address space of the associated value. \p NoAddressSpace is
----------------
Checkout `isValidIRPositionForInit` in upstream attributes. Use it to restrict this one.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:11875
+ "Associated value is not a pointer");
+ A.getAAFor<AAIsDead>(*this, getIRPosition(), DepClassTy::OPTIONAL);
+ }
----------------
No need for AAIsDead.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:11881-11884
+ if (!AUO->getState().isValidState())
+ return takeAddressSpace(getAssociatedType()->getPointerAddressSpace())
+ ? ChangeStatus::CHANGED
+ : ChangeStatus::UNCHANGED;
----------------
No need. Most attributes behave correct w/o state check. This one does too.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:11892-11898
+ if (AssumedAddressSpace == NoAddressSpace) {
+ (void)takeAddressSpace(AS);
+ return true;
+ }
+ assert(AssumedAddressSpace >= 0 &&
+ "AssumedAddressSpace should not be negative");
+ return AS == static_cast<uint32_t>(AssumedAddressSpace);
----------------
Do all of this is takeAS, no need to have the logic split. It should return true if we are still "valid", and false otherwise. You can just do `return takeAS(AS)`
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:11914
+ getAssociatedType()->getPointerAddressSpace())
+ return ChangeStatus::UNCHANGED;
+
----------------
Technically, you can replace the value with poison if you have NoAS at the end.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:11923-11924
+ auto Pred = [&](const Use &U, bool &) {
+ if (U.get() != AssociatedValue)
+ return true;
+ User *Usr = U.getUser();
----------------
Why check this? What can check is if the pointer is actually an AS cast we can just "undo".
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:12004
+ ChangeStatus updateImpl(Attributor &A) override {
+ return ChangeStatus::UNCHANGED;
+ };
----------------
or put this into the initialize of all the ones we do not actually support.
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