[PATCH] D83744: [Attributor] Attributor call site specific AAValueConstantRange

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM with some comments to be addressed. This is cool :)



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:530
+  assert((Pos.getPositionKind() == IRPosition::IRP_ARGUMENT) &&
+         "Expected a 'argument' position !");
+  const CallBase *CBContext = Pos.getCallBaseContext();
----------------
Nit: `an 'argument'`


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:536
+  if (ArgNo < 0)
+    return false;
+  Value *Val = CBContext->getArgOperand(ArgNo);
----------------
This should not happen, right? Make it an assert instead.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:541
+  const auto &AA =
+      A.getAAFor<AAType>(QueryingAttribute, IRPosition::value(*Val));
+  const StateType &CBArgumentState =
----------------
We don't want the value here, do we. We want 

```
  const auto &AA =
      A.getAAFor<AAType>(QueryingAttribute, IRPosition::callsite_argument(*CBContext, ArgNo));
```
which might be better than the plain value result as it has a context instruction (and the attached attributes).


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:602
 
-    IRPosition FnPos = IRPosition::returned(*AssociatedFunction);
+    CallBase *CBContext = (CallBase *)&this->getAnchorValue();
+    if (IntroduceCallBaseContext)
----------------
Use `cast<..>` and keep it as reference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83744/new/

https://reviews.llvm.org/D83744