[PATCH] D23736: CodeGen: Remove MachineFunctionAnalysis => Enable (Machine)ModulePasses

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 22 11:05:09 PDT 2016


MatzeB added inline comments.

================
Comment at: include/llvm/CodeGen/MachineModuleInfo.h:191
@@ +190,3 @@
+  MachineFunctionInitializer *MFInitializer;
+  /// Maps IR Functions to their corresponding MachineFunctions.
+  DenseMap<const Function*, std::unique_ptr<MachineFunction>> MachineFunctions;
----------------
chandlerc wrote:
> And it owns the machine functions.
> 
> Doesn't this make it hard to iterate the machine functions in a deterministic order?
> 
> Would it be better to have a vector of unique pointers for ownership and iteration, and a map from raw pointer to raw pointer for lookup? (genuine question, you know this area much better.
Indeed this should not be used for iteration to avoid indeterministic order.
Right now nobody is using the iteration functionality so this is just a minimal set of functionality to get things going. It is likely that we will need/add iteration functionality in upcoming patches.

================
Comment at: include/llvm/CodeGen/MachineModuleInfo.h:193-194
@@ +192,4 @@
+  DenseMap<const Function*, std::unique_ptr<MachineFunction>> MachineFunctions;
+  const Function *LastRequest = nullptr; ///< Used for shortcut/cache.
+  MachineFunction *LastResult = nullptr; ///< Used for shortcut/cache.
+
----------------
chandlerc wrote:
> Is this really that hot? It seems a risky technique...
To be honest I don't know, quite possibly not. I did not want to slow things down compared what we get today with the MachineFunctionAnalysis, anyway I'll run some tests and remove this if I can't measure a difference even in examples with lots of small functions.


Repository:
  rL LLVM

https://reviews.llvm.org/D23736





More information about the llvm-commits mailing list