[PATCH] D22225: [x86, SSE] optimize pcmp results better (PR28484)

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 11 12:53:53 PDT 2016


spatel added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:28139
@@ +28138,3 @@
+  SDValue Op1 = peekThroughBitcasts(N->getOperand(1));
+  if (Op0.getOpcode() != X86ISD::PCMPEQ && Op0.getOpcode() != X86ISD::PCMPGT)
+    return SDValue();
----------------
delena wrote:
> We cam mark nodes like PCMP with AssertSext. And use this marker to simplify AND.
Ah - I didn't know about AssertSext. To use it, we would add one of those nodes any time we create a PCMPEQ/PCMPGT? And then we would check for an AssertSext at this point rather than PCMPEQ/PCMPGT? 

Ok if I add a 'TODO' comment in this patch?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:28148
@@ +28147,3 @@
+  unsigned EltBitWidth = VT0.getScalarType().getSizeInBits();
+  if (VT0 != VT1 || EltBitWidth == 8)
+    return SDValue();
----------------
delena wrote:
> VT0 is always equal to VT1.
I was paranoid that something like this:
          t18: v4i32 = setcc t2, t4, seteq:ch
          t17: v4i32 = BUILD_VECTOR Constant:i32<1>, Constant:i32<1>, Constant:i32<1>, Constant:i32<1>
        t19: v4i32 = and t18, t17
      t8: v2i64 = bitcast t19
      t10: v2i64 = BUILD_VECTOR Constant:i64<4294967297>, Constant:i64<4294967297>
    t11: v2i64 = and t8, t10

might cause the original (v4i32) constant to get folded away; ie, we might have a bitcast on one side of the 'and' but not the other. If this can't possibly happen, then certainly we can remove the check.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:28152
@@ +28151,3 @@
+  // TODO: Is this possible or worthwhile for AVX-512 variants?
+  if (VT0.getSizeInBits() != 128 && VT0.getSizeInBits() != 256)
+    return SDValue();
----------------
delena wrote:
> On AVX-512 (skylake-avx512) the result is in a mask reg, also for 256 and 128 vector inputs.
If it is the mask form, then wouldn't the node be PCMPEQM rather than PCMPEQ?


http://reviews.llvm.org/D22225





More information about the llvm-commits mailing list