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

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 20 00:01:37 PDT 2017


nemanjai added inline comments.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:295
 
+bool PPCInstrInfo::isReallyTriviallyReMaterializable(const MachineInstr &MI,
+                                                     AliasAnalysis *AA) const {
----------------
MatzeB wrote:
> 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.
I would imagine that the comment about the deprecation of the flag comes right out of `include/llvm/CodeGen/MachineInstr.h`:

```
This flag is deprecated, please don't use it anymore.  If this
flag is set, the isReallyTriviallyReMaterializable() method is called to
verify the instruction is really rematable.
```


https://reviews.llvm.org/D34255





More information about the llvm-commits mailing list