[PATCH] D117147: [MLGO] ML Regalloc Eviction Advisor

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp:484
+
+MLEvictAdvisor::MLEvictAdvisor(const MachineFunction &MF, const RAGreedy &RA,
+                               MLModelRunner *Runner,
----------------
uabelho wrote:
> mtrofin wrote:
> > uabelho wrote:
> > > Hello!
> > > WHen compiling current trunk, 385f5c4d33, this function seems to be unused:
> > > ```
> > > [482/2378] Building CXX object lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/MLRegallocEvictAdvisor.cpp.o
> > > ../lib/CodeGen/MLRegallocEvictAdvisor.cpp:506:1: warning: '{anonymous}::MLEvictAdvisor::MLEvictAdvisor(const llvm::MachineFunction&, const llvm::RAGreedy&, llvm::MLModelRunner*, const llvm::MachineBlockFrequencyInfo&, const llvm::MachineLoopInfo&)' defined but not used [-Wunused-function]
> > >   506 | MLEvictAdvisor::MLEvictAdvisor(const MachineFunction &MF, const RAGreedy &RA,
> > >       | ^~~~~~~~~~~~~~
> > > ```
> > > Will it be used or may it be removed?
> > The use is under conditional compilation. If the warning is causing an issue, I can find an alternative (either wrap the def in  conditional compilation, or have a default use)
> Aha, I see.
> It's not a a major issue, but we compile the codebase with gcc as well as with clang, and gcc complained about it.
> But if the only use is hidden behind an ifdef maybe the function should be as well then?
Ack - or I think I can have a default use. I'll address this shortly (PST here :) )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117147