[PATCH] ARM FastISel integer sext/zext improvements

[JF Bastien]

I fixed my keyboard and sprung for some English, here's an updated patch.
:-)


On Wed, Jun 5, 2013 at 11:29 AM, Eric Christopher <echristo at gmail.com>wrote:

> This will work, some comments on the patch itself:
>
> +  using namespace ARM;
>
> We don't really do this anywhere else...
>
> +  static const uint8_t isSingleInstrTbl[3][2][2][2] = {
> +  static const TargetRegisterClass *RCTbl[2][2] = {
> +  static const struct {
>
> Large explanatory block comments before each of these tables please.
>
> +  unsigned SrcBits = SrcVT.getSizeInBits();
> +  unsigned DestBits = DestVT.getSizeInBits();
> +  assert(SrcBits < DestBits);
> +  assert(DestBits == 32 || DestBits == 16 || DestBits == 8);
> +  assert(SrcBits == 16 || SrcBits == 8 || SrcBits == 1);
> +  unsigned Bitness = countTrailingZeros(SrcBits) >> 1;  //
> {1,8,16}=>{0,1,2}
> +  assert(Bitness < 3);
> +  bool hasV6Ops = Subtarget->hasV6Ops();
> +  bool isSingleInstr =
> isSingleInstrTbl[Bitness][isThumb2][hasV6Ops][isZExt];
> +  const TargetRegisterClass *RC = RCTbl[isThumb2][isSingleInstr];
> +  unsigned Opc = OpcTbl[isSingleInstr][isThumb2][Bitness].Opc[isZExt];
> +  assert(KILL != Opc && "Invalid table entry");
> +  unsigned Imm = OpcTbl[isSingleInstr][isThumb2][Bitness].Imm[isZExt];
> +  unsigned hasS = OpcTbl[isSingleInstr][isThumb2][Bitness].hasS[isZExt];
> +  bool setsCPSR = &tGPRRegClass == RC;  // Always on 16-bit Thumb
> instructions.
> +  unsigned LSLOpc = isThumb2 ? tLSLri : LSLi;
> +  unsigned ResultReg;
>
> Is your return key broken? ;)
>
> Anyhow, some spacing here would be nice along with with a comment on
> each of the asserts (or even text in the assert that says what it's
> checking for ala the other asserts in llvm).
>
> +  for (unsigned Instr = 0; Instr != (isSingleInstr ? 1 : 2); ++Instr) {
>
> Brief comment here as well.
>
> Thanks!
>
> -eric
>
> On Tue, Jun 4, 2013 at 6:06 PM, JF Bastien <jfb at google.com> wrote:
> > My recent ARM FastISel patch exposed this bug:
> >   http://llvm.org/bugs/show_bug.cgi?id=16178
> > The root cause is that it can't select integer sext/zext pre-ARMv6 and
> > asserts out.
> >
> > The current integer sext/zext code doesn't handle other cases gracefully
> > either, so this patch makes it handle all sext and zext from i1/i8/i16 to
> > i8/i16/i32, with and without ARMv6, both in Thumb and ARM mode. This
> should
> > fix the bug as well as make FastISel faster because it bails to
> SelectionDAG
> > less often. See fastisel-ext.patch for this.
> >
> > fastisel-ext-tests.patch changes current tests to always use reg-imm AND
> for
> > 8-bit zext instead of UXTB. This simplifies code since it is supported on
> > ARMv4t and later, and at least on A15 both should perform exactly the
> same
> > (both have exec 1 uop 1, type I).
> >
> > 2013-05-31-char-shift-crash.ll is a bitcode version of the above bug
> 16178
> > repro.
> >
> > fast-isel-ext.ll tests all sext/zext combinations that ARM FastISel
> should
> > now handle.
> >
> > Note that my ARM FastISel enabling patch was reverted due to a separate
> > failure when dealing with MCJIT, I'll fix this second failure and then
> turn
> > FastISel on again for non-iOS ARM targets.
> >
> > I've tested "make check-all" on my x86 box, and "lnt test-suite" on A15
> > hardware.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130605/e771bd28/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fastisel-ext.patch
Type: application/octet-stream
Size: 7009 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130605/e771bd28/attachment.obj>