[X86] Simplify cmp-zext-constant DAG patterns.

[bryant]

Summary

This cuts down on the spurious generation of MOV32rr and should be correct for the E, NE, P, NP condition codes. However, I am unsure how useful this might be since movl uses zero micro-ops.

I've added nadav since s/he appears in grep -i x86 CODE_OWNERS.txt.

[bryant]

Added test case.

[bryant]

Revert accident deletion

[RKSimon]

test/CodeGen/X86/cmp-zext-combine.ll

22

Please can you simplify the test to focus just on the combine in question. And preferably add a wider range of tests touching different comparisons / integer sizes (inc ones that shouldn't combine).

If possible use utils/update_llc_test_checks.py to generate full code output (you will need to add check prefixes to the 32/64 tests)

[spatel]

lib/Target/X86/X86ISelLowering.cpp

30277–30282 ↗

(On Diff #64876)

Usually (always?) you would start at the outer op/instruction/node of a pattern that you want to match. Is there some reason not to implement this starting in combineSetCC() rather than combineX86Cmp()?

[spatel]

Just noticed that llvm-commits is not subscribed to this review. It's probably best to abandon this review and start again with llvm-commits subscribed from the outset.

[bryant]

lib/Target/X86/X86ISelLowering.cpp

30277–30282 ↗

(On Diff #64876)

• in order for this transformation to occur, there must be an ISD::ZERO_EXTEND present in the inner node.

• often, however, the zext doesn't appear until DAGCombiner generates it somewhere down the line in the combine process -- after the corresponding parent setcc has been touched by combineSetCC/combineX86SetCC;
  • ISD::ZERO_EXTEND is set to Expand in this file, although it won't matter much since there are many other patterns that need DAGCombiner to distill into a zext.

• when DAGCombiner combines, it re-adds to the work list only the direct users of the zext node. this would be the cmp but not the setcc.

[bryant]

Upon closer inspection, I've discovered that the true root of the problem lies in DAGCombiner failing to recognize and combine (srl (shl, c), c). So I will be re-submitting the fix there instead.

[RKSimon]

Abandon this given D22726?

[bryant]

yes.

test/CodeGen/X86/cmp-zext-combine.ll

+; RUN: llc -mtriple=x86_64 < %s | FileCheck %s
+; RUN: llc -mtriple=i686 < %s | FileCheck %s
+
+define i32 @f(i64*, i32, i32) {
+; CHECK-LABEL: f
+  %4 = alloca i64*, align 8
+  %5 = bitcast i64** %4 to i8*
+  store i64* %0, i64** %4, align 8
+  %6 = call { i32, i32 } asm sideeffect "cmpxchg8b $2", "={ax},={dx},=*m,{bx},{cx},0,1,*m,~{flags},~{dirflag},~{fpsr},~{flags}"(i64** nonnull %4, i32 4, i32 5, i32 0, i32 0, i64** nonnull %4) #5
+  %7 = extractvalue { i32, i32 } %6, 0
+  %8 = extractvalue { i32, i32 } %6, 1
+  %9 = zext i32 %8 to i64
+  %10 = shl nuw i64 %9, 32
+; CHECK-DAG: testl %eax, %eax
+  %11 = icmp ne i32 %7, 0
+; CHECK-DAG: testl %edx, %edx
+  %12 = icmp ugt i64 %10, 4294967295
+  %13 = and i1 %11, %12
+  %14 = zext i1 %13 to i32
+  ret i32 %14
+}