[PATCH] D157358: [FunctionPropertiesAnalysis] Add detailed analysis
Aiden Grossman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 8 17:37:59 PDT 2023
aidengrossman added inline comments.
================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:90
+
+ unsigned PredecessorCount = succ_size(&BB);
+ if (PredecessorCount == 1)
----------------
mtrofin wrote:
> `pred_size`
>
> Is the test catching this?
Tests were not currently catching that. I've added a new unit test case to make sure the behavior is as expected when the number of successors and predecessors is different.
================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:105
+
+ for (const Instruction &I : BB) {
+ if (I.isCast())
----------------
mtrofin wrote:
> probably want to ignore debug instructions
>
> also, do you care if it's an intrinsic?
Not currently. I'm probably going to add a couple more features in the future and a count of instructions that are intrinsics would be nice to have, but currently focused on getting the original feature set in.
================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:119
+ if (C->getType()->isIntegerTy())
+ IntegerConstantCount += 1;
+ else if (C->getType()->isFloatTy())
----------------
mtrofin wrote:
> `++IntegerConstantCount`?
I've switched to doing `+= Direction` so that this better matches the behavior of the rest of the function when updating a function analysis. It doesn't really matter in my case, but definitely shouldn't purposefully be different.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157358/new/
https://reviews.llvm.org/D157358
More information about the llvm-commits
mailing list