[PATCH] D80249: CodeGen: Don't lazily construct MachineFunctionInfo

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 accepted this revision.
barannikov88 added a comment.

Few nitpicks and LGTM.
Ran into this once or twice as well



================
Comment at: llvm/include/llvm/CodeGen/MachineFunction.h:104
+  static Ty *create(BumpPtrAllocator &Allocator, const Function &F,
+                    const TargetSubtargetInfo *STI) {
+    return new (Allocator.Allocate<Ty>()) Ty(F, STI);
----------------
Should STI be also passed by reference (can it be null)?


================
Comment at: llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h:195
 public:
-  explicit AArch64FunctionInfo(MachineFunction &MF);
+  explicit AArch64FunctionInfo(const Function &F,
+                               const TargetSubtargetInfo *STI);
----------------
`explicit` is now redundant. (Here and in other backends.)


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:846
+    BumpPtrAllocator &Allocator, const Function &F,
+    const TargetSubtargetInfo *STI) const {
+  return AArch64FunctionInfo::create<AArch64FunctionInfo>(Allocator, F, STI);
----------------
It would be natural to pass AArch64FunctionInfo here instead of TargetSubtargetInfo. Like in AMDGPU backend below.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:21
+                                             const AMDGPUSubtarget &ST)
+    : MachineFunctionInfo(),
+      IsEntryFunction(
----------------
Calling constructor without arguments does not seem necessary...


================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:39
+SIMachineFunctionInfo::SIMachineFunctionInfo(const Function &F,
+                                             const TargetSubtargetInfo *STI)
+  : AMDGPUMachineFunction(F, static_cast<const GCNSubtarget &>(*STI)),
----------------
Passing GCNSubtarget instead of TargetSubtargetInfo would save a cast :)


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

https://reviews.llvm.org/D80249