[PATCH] D40497: [PowerPC] Partially enable the ISEL expansion pass.

Tony Jiang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 28 10:45:41 PST 2017


jtony marked 5 inline comments as done.
jtony added inline comments.


================
Comment at: lib/Target/PowerPC/PPCExpandISEL.cpp:279
     // Special case 2, the two input registers used by ISEL are the same.
     // Note 1: We favor merging ISEL expansions over folding a single one. If
     // the passed list has multiple merge-able ISEL's, we won't fold any.
----------------
syzaara wrote:
> jtony wrote:
> > syzaara wrote:
> > > It seems that Note 1 is no longer satisfied since you have already folded the ISELs in expandAndMergeISELs.
> > Same here.
> If we have the case of:
> foldable isel
> foldable isel
> regular isel
> 
> where both the foldable isels can be merged, previously with expansion enabled, we would merge them. Now, with the expansion enabled, they will be changed to move registers before. So we won't be getting the same behavior with the expansion enabled, making this comment invalid.
That's right Zaara. We could guard the first two special cases handling inside expandAndMergeISELs() with condition `if (!ExpandISELEnabled)` if we really want to keep the the behavior in the original implementation. Or If we are OK for the change, we just need to update the comments here. @nemanjai  Do you have any comments? 


https://reviews.llvm.org/D40497





More information about the llvm-commits mailing list