TableGen does not handle AND masks correctly

[Quuxplusone]

Bugzilla Link	PR35375
Status	RESOLVED FIXED
Importance	P enhancement
Reported by	Diana Picus (diana.picus@linaro.org)
Reported on	2017-11-21 07:22:06 -0800
Last modified on	2020-09-13 16:18:14 -0700
Version	trunk
Hardware	PC Linux
CC	daniel_l_sanders@apple.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, paul@windfall-software.com
Fixed by commit(s)
Attachments	pkhbt.ll (193 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 19455
Reproducer

llc -O0 -mtriple armv7-- -stop-before=expand-isel-pseudos -o - pkhbt.ll
produces a PKHBT instruction, but with -global-isel it does not.

The GlobalISelEmitter for TableGen generates code to select the PKHBT pattern:
def PKHBT : APKHI<0b01101000, 0, (outs GPRnopc:$Rd),
                              (ins GPRnopc:$Rn, GPRnopc:$Rm, pkh_lsl_amt:$sh),
               IIC_iALUsi, "pkhbt", "\t$Rd, $Rn, $Rm$sh",
               [(set GPRnopc:$Rd, (or (and GPRnopc:$Rn, 0xFFFF),
                                      (and (shl GPRnopc:$Rm, pkh_lsl_amt:$sh),
                                           0xFFFF0000)))]>,
               Requires<[IsARM, HasV6]>,
           Sched<[WriteALUsi, ReadALU]>;

The problem is that it fails when trying to compare -65536 (or 4294901760) to
0xFFFF,0000. This is because the constant in the instruction is sign extended
to 64 bits (0xFFFF,FFFF,FFFF,0000) and then compared to the non-extended 64 bit
version expected by TableGen.

In contrast, the DAGISelEmitter generates special code for AND immediates
(OPC_CheckAndImm), which does not sign extend.

[Quuxplusone]

Attached pkhbt.ll (193 bytes, text/plain): Reproducer

[Quuxplusone]

The SelectionDAG code for this is a bit odd. I'm surprised that and/or are covered by special cases but xor isn't. It's also surprising that always-true predicates and powers of 2 (including 0x80000000) prevent the special case.

It sounds like the right thing to do is to only check the bits that are actually present in the LLT (similar to SelectionDAG's use of APInt). In this case, the 0xffff0000 and the s32 type should be treated as check for (X & 0xffffffff) == (0xffff0000 & 0xffffffff). We should probably have a warning if the pattern uses a constant that isn't an extension like 0x1000ffff with s16.

I'll take a look at the reproducer

[Quuxplusone]

I've looked into it a bit more and I've ended up fixing it by sign-extending the table value to match the sign-extension that getConstantVRegVal() will perform.

https://reviews.llvm.org/D40532