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

[Craig Topper via Phabricator via llvm-commits]

craig.topper 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;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104612