[llvm] r319252 - [globalisel][tablegen] Fix PR35375 by sign-extending the table value to match getConstantVRegVal()

[Daniel Sanders via llvm-commits]

Author: dsanders
Date: Tue Nov 28 15:18:54 2017
New Revision: 319252

URL: http://llvm.org/viewvc/llvm-project?rev=319252&view=rev
Log:
[globalisel][tablegen] Fix PR35375 by sign-extending the table value to match getConstantVRegVal()

Summary:
>From the bug report:
> 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.

This patch doesn't introduce the special case for AND (and OR) immediates since the majority of it is related to handling known bits that have no effect on the result and GlobalISel doesn't detect known-bits at this time. Instead this patch just ensures that the immediate is extended consistently on both sides of the check.

Thanks to Diana Picus for the detailed bug report.

Reviewers: rovka

Reviewed By: rovka

Subscribers: kristof.beyls, javed.absar, llvm-commits

Differential Revision: https://reviews.llvm.org/D40532

Added:
    llvm/trunk/test/CodeGen/ARM/GlobalISel/pr35375.ll
Modified:
    llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h?rev=319252&r1=319251&r2=319252&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h Tue Nov 28 15:18:54 2017
@@ -337,6 +337,11 @@ bool InstructionSelector::executeMatchTa
                              << InsnID << "]->getOperand(" << OpIdx
                              << "), Value=" << Value << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
+
+      // isOperandImmEqual() will sign-extend to 64-bits, so should we.
+      LLT Ty = MRI.getType(State.MIs[InsnID]->getOperand(OpIdx).getReg());
+      Value = SignExtend64(Value, Ty.getSizeInBits());
+
       if (!isOperandImmEqual(State.MIs[InsnID]->getOperand(OpIdx), Value,
                              MRI)) {
         if (handleReject() == RejectAndGiveUp)

Added: llvm/trunk/test/CodeGen/ARM/GlobalISel/pr35375.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/GlobalISel/pr35375.ll?rev=319252&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/GlobalISel/pr35375.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/GlobalISel/pr35375.ll Tue Nov 28 15:18:54 2017
@@ -0,0 +1,12 @@
+; RUN: llc -O0 -mtriple armv7-- -stop-before=expand-isel-pseudos < %s
+; RUN: llc -O0 -mtriple armv7-- -stop-before=expand-isel-pseudos -global-isel < %s
+
+; CHECK: PKHBT
+
+define arm_aapcscc i32 @pkh(i32 %x, i32 %y) {
+  %andx = and i32 %x, 65535
+  %shl = shl i32 %y, 1
+  %andy = and i32 %shl, 4294901760 ; same as -65536
+  %or = or i32 %andx, %andy
+  ret i32 %or
+}