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

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 23 16:20:20 PDT 2018


MatzeB added inline comments.


================
Comment at: test/CodeGen/X86/pr38771.ll:8
 ; CHECK-NEXT:    movl $0, (%rax)
+; CHECK-NEXT:    movq $0, (%rax)
 ; CHECK-NEXT:    retq
----------------
craig.topper wrote:
> 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.
Thanks for looking into this! I guess I will go ahead and drop the testcase as part of the commit then.


Repository:
  rL LLVM

https://reviews.llvm.org/D53181





More information about the llvm-commits mailing list