[PATCH] D53181: SelectionDAG: Reuse bigger sized constants in memset expansion.

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 11 22:22:25 PDT 2018


craig.topper added inline comments.


================
Comment at: test/CodeGen/X86/pr38771.ll:8
 ; CHECK-NEXT:    movl $0, (%rax)
+; CHECK-NEXT:    movq $0, (%rax)
 ; CHECK-NEXT:    retq
----------------
jfb wrote:
> MatzeB wrote:
> > This code changes because ConstantHoisting touches the constants here, and the less aggressive behavior of trunc(OpaqueConstant)" somehow makes DAGCombiner realizes this whole expression being 0 now which it didn't do before. The testcase unfortunately doesn't have enough information to figure out how an equivalent tests would look like that wouldn't constant fold to zero. So this probably has to stay like this...
> I expect @craig.topper can chime in on this one.
I'm having trouble reducing to a better test case.

The existing test case creates an 'and' while legalizing a srl_parts with an opaque constant for a shift quantity. This failed an assert later in the same legalization. That shift was already sketchy because it used a shift amount that was larger than the bit.

I tried to just make an and with two opaque constant operands. Constant folding in visitAND in DAG combine obeys the opaqueness, but SimplifyDemandedBits called later in visitAND does not. So DAG combine simplifies it when it probably shouldn't.

I'm not very concerned about losing this test case. It's only verifying an assert. There was no code changes associated with it.


Repository:
  rL LLVM

https://reviews.llvm.org/D53181





More information about the llvm-commits mailing list