[PATCH] D54710: [x86] don't try to convert add with undef operands to LEA

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: craig.topper, MatzeB, dsanders.
Herald added a subscriber: mcrosier.

There's existing code here that tries to handle an undef operand while transforming an add to an LEA, but it's incomplete because we will crash on the i16 test with the debug output shown below. I think it's better to just give up instead. Really, GlobalIsel should have folded these before we could get into trouble?

Machine code for function add_undef_i16: NoPHIs, TracksLiveness, Legalized, RegBankSelected, Selected
=====================================================================================================

bb.0 (%ir-block.0):

  liveins: $edi
  %1:gr32 = COPY killed $edi
  %0:gr16 = COPY %1.sub_16bit:gr32
  %5:gr64_nosp = IMPLICIT_DEF
  %5.sub_16bit:gr64_nosp = COPY %0:gr16
  %6:gr64_nosp = IMPLICIT_DEF
  %6.sub_16bit:gr64_nosp = COPY %2:gr16
  %4:gr32 = LEA64_32r killed %5:gr64_nosp, 1, killed %6:gr64_nosp, 0, $noreg
  %3:gr16 = COPY killed %4.sub_16bit:gr32
  $ax = COPY killed %3:gr16
  RET 0, implicit killed $ax

End machine code for function add_undef_i16.
============================================

  - Bad machine code: Reading virtual register without a def ***
- function:    add_undef_i16
- basic block: %bb.0  (0x7fe6cd83d940)
- instruction: %6.sub_16bit:gr64_nosp = COPY %2:gr16
- operand 1:   %2:gr16

LLVM ERROR: Found 1 machine code errors.


https://reviews.llvm.org/D54710

Files:
  lib/Target/X86/X86InstrInfo.cpp
  test/CodeGen/X86/GlobalISel/undef.ll


Index: test/CodeGen/X86/GlobalISel/undef.ll
===================================================================
--- test/CodeGen/X86/GlobalISel/undef.ll
+++ test/CodeGen/X86/GlobalISel/undef.ll
@@ -1,15 +1,15 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=x86_64-linux-gnu            -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL
+; RUN: llc -mtriple=x86_64-linux-gnu -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL
 
 define i8 @test() {
 ; ALL-LABEL: test:
 ; ALL:       # %bb.0:
 ; ALL-NEXT:    retq
   ret i8 undef
 }
 
-define i8 @test2(i8 %a) {
-; ALL-LABEL: test2:
+define i8 @add_undef_i8(i8 %a) {
+; ALL-LABEL: add_undef_i8:
 ; ALL:       # %bb.0:
 ; ALL-NEXT:    movl %edi, %eax
 ; ALL-NEXT:    addb %al, %al
@@ -19,6 +19,26 @@
   ret i8 %r
 }
 
+define i16 @add_undef_i16(i16 %a) {
+; ALL-LABEL: add_undef_i16:
+; ALL:       # %bb.0:
+; ALL-NEXT:    movl %edi, %eax
+; ALL-NEXT:    addw %ax, %ax
+; ALL-NEXT:    # kill: def $ax killed $ax killed $eax
+; ALL-NEXT:    retq
+  %r = add i16 %a, undef
+  ret i16 %r
+}
+
+define i32 @add_undef_i32(i32 %a) {
+; ALL-LABEL: add_undef_i32:
+; ALL:       # %bb.0:
+; ALL-NEXT:    movl %edi, %eax
+; ALL-NEXT:    addl %eax, %eax
+; ALL-NEXT:    retq
+  %r = add i32 %a, undef
+  ret i32 %r
+}
 
 define float @test3() {
 ; ALL-LABEL: test3:
Index: lib/Target/X86/X86InstrInfo.cpp
===================================================================
--- lib/Target/X86/X86InstrInfo.cpp
+++ lib/Target/X86/X86InstrInfo.cpp
@@ -926,6 +926,14 @@
   const MachineOperand &Dest = MI.getOperand(0);
   const MachineOperand &Src = MI.getOperand(1);
 
+  // Ideally, operations with undef should be folded before we get here, but we
+  // can't guarantee it. Bail out because optimizing undefs is a waste of time.
+  if (Src.isUndef())
+    return nullptr;
+  if (MI.getNumOperands() > 2)
+    if (MI.getOperand(2).isReg() && MI.getOperand(2).isUndef())
+      return nullptr;
+
   MachineInstr *NewMI = nullptr;
   // FIXME: 16-bit LEA's are really slow on Athlons, but not bad on P4's.  When
   // we have better subtarget support, enable the 16-bit LEA generation here.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54710.174631.patch
Type: text/x-patch
Size: 2235 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181119/bf266287/attachment.bin>