[PATCH] D76175: [Attributor][NFC] Typos in doc

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:168
     IRP_CALL_SITE_ARGUMENT = 1, ///< An attribute for a call site argument.
   };
 
----------------
baziotis wrote:
> jdoerfert wrote:
> > FWIW. The encoding allows to check for arguments by doing `value >= 0`. If that is true, the argument number is the value. 
> But how do you know if it's a call site argument or a formal parameter (i.e. `IRP_ARGUMENT`) ?
> Since argument numbers start from 0, this is problematic (e.g. see the previous comment regarding the `argument()` constructor).
> Plus then there are a lot of `switch`es that if say we have argument number == 3 don't seem to be that clear.
> I'm mainly asking in order to be able to understand the current code so that I can change it.
If the value is smaller than 0, it is of the type specified by the value, thus the value is the "kind".
If the value is at least 0 and the associated value is an llvm::Argument, it is an argument at position value.
If the value is at least 0 and the associated value is an llvm::CallBase, it is a call site argument at position value.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:280
            "Invalid position does not have an anchor scope!");
-    Value &V = getAnchorValue();
-    if (isa<Function>(V))
-      return &cast<Function>(V);
-    if (isa<Argument>(V))
-      return cast<Argument>(V).getParent();
-    if (isa<Instruction>(V))
-      return cast<Instruction>(V).getFunction();
-    return nullptr;
+    return getAnchorScope();
   }
----------------
baziotis wrote:
> sstefan1 wrote:
> > This makes sense. Now that I look at the other parts we seem to mix these two a lot. I think the better solution is to inline `getAnchorScope` here and replace all the call sites with `getAssociatedFunction` as it's clearer (at least to me). 
> Yes, I like that too.
These are different things for call site [arguments]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76175