[PATCH] D63590: [PowerPC] Sign extend the select instr operands if it is any_extend

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang marked an inline comment as done.
steven.zhang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:12325
       else
-        Ops[C+i] = DAG.getAnyExtOrTrunc(Ops[C+i], dl, N->getValueType(0));
+        llvm_unreachable("Unexpected opcode!");
     }
----------------
hfinkel wrote:
> steven.zhang wrote:
> > hfinkel wrote:
> > > This seems like we're working around a problem elsewhere. Why does forcing any_extend to become a sign_extend produce better code than just producing an any_extend?
> > For any_extend, it is handled as zero extend if it is constant. For example, 
> > ```
> > t1: i8 = select t0, Constant:i8<-1>, Constant:i8<0>
> > t2: i64 = any_extend t1
> > ```
> > It  will be combined as follows, for now:
> > ```
> > t3: i64 = select t0, Constant:i64<255>, Constant:i64<0>
> > ```
> > And the literal "255" is just a normal constant, therefore, the combine to inreg rule won't apply. This is what we expected:
> > ```
> > t3: i64 = select t0, Constant:i64<-1>, Constant:i64<0>
> > ```
> > So that, the combine to inreg rule would apply and it is combined as:
> > t4: i64 = sign_extend_inreg t3
> > 
> > Doing zero extend will hurt some opportunities, while sign extend indeed helps. And we haven't seen the downside yet.
> > Doing zero extend will hurt some opportunities, while sign extend indeed helps. And we haven't seen the downside yet.
> 
> What happens if we always prefer to lower any_extend to sign_extend instead of just doing it in this special case? Given that, on PPC, immediates are generally sign extended, etc., maybe that should be the default?
> 
Yes, I tried. For powerpc, I didn't see any problems by doing this, but also no benefit from the sanity test and benchmark 2017, but for X86, I indeed see many bad changes. I guess they have some assumption on this. So, I decide to just do it for aext + select. 


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

https://reviews.llvm.org/D63590