[PATCH] ARM FastISel integer sext/zext improvements

[Eric Christopher]

Sure. Have at.

-eric

On Wed, Jun 5, 2013 at 6:04 PM, JF Bastien <jfb at google.com> wrote:
> 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.
>
>