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

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added inline comments.


================
Comment at: llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp:484
+
+MLEvictAdvisor::MLEvictAdvisor(const MachineFunction &MF, const RAGreedy &RA,
+                               MLModelRunner *Runner,
----------------
mtrofin wrote:
> 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 :) )
Nice, thanks! No panic :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117147