[PATCH] D34255: [PowerPC] define target hook isReallyTriviallyReMaterializable

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 23:00:43 PDT 2017


MatzeB added inline comments.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:295
 
+bool PPCInstrInfo::isReallyTriviallyReMaterializable(const MachineInstr &MI,
+                                                     AliasAnalysis *AA) const {
----------------
echristo wrote:
> MatzeB wrote:
> > lei wrote:
> > > echristo wrote:
> > > > This needs a comment as to why each of these need to be marked as rematerializable here rather than in the .td file.
> > > Will add:
> > > 
> > > // The Rematerializable flag is deprecated and if it is set, isReallyTriviallyReMaterializable() method need to be called to verify the instruction is really rematable.
> > Why do you get the impression that this flag is deprecated? It's just not the only thing checked as far as I can see.
> Yeah. I was wondering about that. Also, that's really confusing. The routine itself could use a comment update explaining what's going on? Or at least it wasn't horribly clear to me.
Indeed that is one of the many things in CodeGen that are really confusing (and unnecessarily so it seems). However stating it is deprecated will only add to to the confusion unless there are concrete plans to do something about it.

And I'd certainly welcome a change that removes the MCInstrDesc bit and only uses the callback and measures that it is indeed negligible in terms of compiletime.


https://reviews.llvm.org/D34255





More information about the llvm-commits mailing list