================
@@ -148,17 +184,9 @@ bool 
StaticDataSplitter::partitionStaticDataWithProfiles(MachineFunction &MF) {
 
           if (MJTI->updateJumpTableEntryHotness(JTI, Hotness))
             ++NumChangedJumpTables;
-        } else {
-          // Find global variables with local linkage.
-          const GlobalVariable *GV =
-              getLocalLinkageGlobalVariable(Op.getGlobal());
-          // Skip 'special' global variables conservatively because they are
-          // often handled specially, and skip those not in static data
-          // sections.
-          if (!GV || GV->getName().starts_with("llvm.") ||
-              !inStaticDataSection(GV, TM))
-            continue;
-          SDPI->addConstantProfileCount(GV, Count);
+        } else if (const Constant *C =
+                       getConstant(Op, MF.getTarget(), MF.getConstantPool())) {
+          SDPI->addConstantProfileCount(C, Count);
----------------
mingmingl-llvm wrote:

Good question. Or rather, why do StaticDataSplitter pass return 'true' or 
'false` to pass manager? 

Notably, 
1) For the StaticdataSplitter pass, none of the data structures 
(MachineJumpTableInfo or immutable pass StaticDataProfileInfoWrapperPass) 
touched by this pass are tracked by its mutable, required analysis, and by 
definition of pass manager, it means it's _correct_ to say call 
[setPreservesAll](https://github.com/llvm/llvm-project/blob/ecaef010f31e2557d94b4d98774ca4b4d5fe2149/llvm/include/llvm/PassAnalysisSupport.h#L129-L130)
 for this pass. 
2) For faster compile time, we want subsequent passes to reuse analysis result 
(of mutable passes) as much as possible rather than re-computing them, since 
the analysis result are not mutated by this pass. Also pass manager won't 
automatically re-schedule (or re-compute) an immutable pass whether a 
transformation pass returns true or false.

I think the return value is mainly for informative purposes. More specifically, 
pass manager can dump [2] pass execution history (presumably when people are 
debugging something), when 
1) `-debug-pass` is specified as `Executions` or higher [1] (e.g., for `opt`, 
`llc` or `clang -mllvm -debug-pass=<xxx>`)
2) a pass return true which makes 
`[LocalChanged](https://github.com/llvm/llvm-project/blob/ecaef010f31e2557d94b4d98774ca4b4d5fe2149/llvm/lib/IR/LegacyPassManager.cpp#L1406)`
 true,

Upon this comment and discussion, I made two code changes (along with comments)
1) In `getAnalysisUsage` (at line 87 - 97) to call `setPreservesAll()`
2) In `partitionStaticDataWithProfiles`, changed `int NumChangedJumpTables` to 
`bool Changed`, and make it track both jump table and constant changes. How 
does this look?

[1] 
https://github.com/llvm/llvm-project/blob/ecaef010f31e2557d94b4d98774ca4b4d5fe2149/llvm/lib/IR/LegacyPassManager.cpp#L1152-L1153
[2] 
https://github.com/llvm/llvm-project/blob/ecaef010f31e2557d94b4d98774ca4b4d5fe2149/llvm/lib/IR/LegacyPassManager.cpp#L1433-L1434

https://github.com/llvm/llvm-project/pull/129781
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to