[PATCH] D38771: [x86] avoid infinite loop from SoftenFloatOperand (PR34866)

[Sanjay Patel via Phabricator via llvm-commits]

spatel updated this revision to Diff 118601.
spatel added a comment.

Patch updated:
Use peekThroughBitcasts() to make the code shorter and more robust.

Note:
I found this as the source of the MMX requirement, but I'm still not sure what is expected from these tests (other than not crashing):

  // Long double always uses X87, except f128 in MMX.
  if (UseX87) {
    if (Subtarget.is64Bit() && Subtarget.hasMMX()) {
      addRegisterClass(MVT::f128, &X86::FR128RegClass);
      ValueTypeActions.setTypeAction(MVT::f128, TypeSoftenFloat);

...for the no-mmx target, we would generate:

  movq	%rsi, %xmm0
  movq	%rdi, %xmm1
  punpcklqdq	%xmm0, %xmm1    ## xmm1 = xmm1[0],xmm0[0]
  pxor	%xmm0, %xmm0
  pcmpeqb	%xmm1, %xmm0
  pmovmskb	%xmm0, %eax
  cmpl	$65535, %eax            ## imm = 0xFFFF
  sete	%al
  retq


https://reviews.llvm.org/D38771

Files:
  lib/Target/X86/X86ISelLowering.cpp
  test/CodeGen/X86/fp128-cast.ll


Index: test/CodeGen/X86/fp128-cast.ll
===================================================================
--- test/CodeGen/X86/fp128-cast.ll
+++ test/CodeGen/X86/fp128-cast.ll
@@ -359,6 +359,58 @@
 ; X64:       retq
 }
 
+; This would inf-loop: https://bugs.llvm.org/show_bug.cgi?id=34866
+
+define i1 @PR34866(i128 %x) {
+; X64-LABEL: PR34866:
+; X64:       # BB#0:
+; X64-NEXT:    movaps {{.*}}(%rip), %xmm0
+; X64-NEXT:    movaps %xmm0, -{{[0-9]+}}(%rsp)
+; X64-NEXT:    xorq -{{[0-9]+}}(%rsp), %rsi
+; X64-NEXT:    xorq -{{[0-9]+}}(%rsp), %rdi
+; X64-NEXT:    orq %rsi, %rdi
+; X64-NEXT:    sete %al
+; X64-NEXT:    retq
+;
+; X32-LABEL: PR34866:
+; X32:       # BB#0:
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    orl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    orl {{[0-9]+}}(%esp), %eax
+; X32-NEXT:    orl %ecx, %eax
+; X32-NEXT:    sete %al
+; X32-NEXT:    retl
+  %bc_mmx = bitcast fp128 0xL00000000000000000000000000000000 to i128
+  %cmp = icmp eq i128 %bc_mmx, %x
+  ret i1 %cmp
+}
+
+define i1 @PR34866_commute(i128 %x) {
+; X64-LABEL: PR34866_commute:
+; X64:       # BB#0:
+; X64-NEXT:    movaps {{.*}}(%rip), %xmm0
+; X64-NEXT:    movaps %xmm0, -{{[0-9]+}}(%rsp)
+; X64-NEXT:    xorq -{{[0-9]+}}(%rsp), %rsi
+; X64-NEXT:    xorq -{{[0-9]+}}(%rsp), %rdi
+; X64-NEXT:    orq %rsi, %rdi
+; X64-NEXT:    sete %al
+; X64-NEXT:    retq
+;
+; X32-LABEL: PR34866_commute:
+; X32:       # BB#0:
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    orl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    orl {{[0-9]+}}(%esp), %eax
+; X32-NEXT:    orl %ecx, %eax
+; X32-NEXT:    sete %al
+; X32-NEXT:    retl
+  %bc_mmx = bitcast fp128 0xL00000000000000000000000000000000 to i128
+  %cmp = icmp eq i128 %x, %bc_mmx
+  ret i1 %cmp
+}
+
 declare double @copysign(double, double) #1
 
 attributes #2 = { nounwind readnone }
Index: lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- lib/Target/X86/X86ISelLowering.cpp
+++ lib/Target/X86/X86ISelLowering.cpp
@@ -35222,6 +35222,11 @@
   if (!OpVT.isScalarInteger() || OpSize < 128 || isNullConstant(Y))
     return SDValue();
 
+  // Bail out if we know that this is not really just an oversized integer.
+  if (peekThroughBitcasts(X).getValueType() == MVT::f128 ||
+      peekThroughBitcasts(Y).getValueType() == MVT::f128)
+    return SDValue();
+
   // TODO: Use PXOR + PTEST for SSE4.1 or later?
   // TODO: Add support for AVX-512.
   EVT VT = SetCC->getValueType(0);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38771.118601.patch
Type: text/x-patch
Size: 2571 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171011/7dc0d97d/attachment.bin>