[PATCH] D104612: [CGP][RISCV] Teach CodeGenPrepare::optimizeSwitchInst to honor isSExtCheaperThanZExt.

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:7014-7016
+  // Some targets prefer SExt over ZExt, honor that.
+  if (TLI->isSExtCheaperThanZExt(OldVT, RegType))
+    ExtType = Instruction::SExt;
----------------
craig.topper wrote:
> jrtc27 wrote:
> > jrtc27 wrote:
> > > Do we not want to avoid this in the case this is a function argument that is already being zero-extended? i.e. something like:
> > > 
> > > ```
> > >   Instruction::CastOps ExtType = Instruction::CastOpsEnd;
> > >   if (auto *Arg = dyn_cast<Argument>(Cond)) {
> > >     if (Arg->hasSExtAttr())
> > >       ExtType = Instruction::SExt;
> > >     else if (Arg->hasZExtAttr())
> > >       ExtType = Instruction::ZExt;
> > >   }
> > >   if (ExtType == Instruction::CastOpsEnd) {
> > >     if (TLI->isSExtCheaperThanZExt(OldVT, RegType))
> > >       ExtType = Instruction::SExt;
> > >     else
> > >       ExtType = Instruction::ZExt;
> > >   }
> > > ```
> > > (slightly abusing CastOpsEnd as being one past the last valid op, and thus a sentinel, though you could write it in other ways too if you wanted)
> > > 
> > Or maybe the fact that sext is cheaper means the saving for each case's extending to match outweighs the cost of having to also extend the argument?
> It's a non-issue for RISCV since I don't think we ever create i32 zext arguments, but it would be more consistent. Could probably just do the isSExtCheaperThanZExt first and then check if it is an Argument after.
True, only i16 and i8, and those aren't so cheap to sext. Your suggestion is indeed the cleaner way to express the same thing, not sure how I missed that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104612