[PATCH] D29687: [TargetLowering] check for sign-bit comparisons in SimplifyDemandedBits

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:770
+      // TODO: Should we check for other forms of sign-bit comparisons?
+      // Examples: X <= -1, X >= 0
+    }
----------------
RKSimon wrote:
> Add test cases for these?
I'm not sure we want to bloat it up with those cases yet; that's why I made it a TODO? rather than a FIXME.
Instcombine already canonicalizes the X <= -1 variant to X < 0. It doesn't know to change X > -1 to X < 0 and swap the select operands, but I think that's just an IR canonicalization oversight, so I was planning to fix that.

So if we add codegen tests for those variants, it's only because a non-canonical select pattern has been created in the backend. I'd like to find evidence of that happening before adding code or tests for it.


================
Comment at: test/CodeGen/X86/vselect-pcmp.ll:4
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx2      | FileCheck %s --check-prefix=AVX --check-prefix=AVX2
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx512f  | FileCheck %s --check-prefix=AVX --check-prefix=AVX512F
 
----------------
RKSimon wrote:
> Please can you add a avx512vl target to check more mask predicate cases?
Sure - I'm still not clear on all of the avx512 variants, but I added a 'vl' RUN line here:
rL294462

I'll rebase with those diffs next.


https://reviews.llvm.org/D29687