<div dir="ltr">This looks reasonable, assuming creating a bunch of intervalmaps is not that expensive.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 29, 2016 at 5:41 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hfinkel created this revision.<br>
hfinkel added reviewers: dberlin, chandlerc, echristo, igor-laevsky, mcrosier, eeckstein.<br>
hfinkel added a subscriber: llvm-commits.<br>
Herald added a subscriber: mcrosier.<br>
<br>
DeadStoreElimination can currently remove a small store rendered unnecessary by a later larger one, but cannot remove a larger store rendered unnecessary by a series of later smaller ones. This patch aims to rectify that.<br>
<br>
It works by keeping an IntervalMap for each store later overwritten only partially, and filling in that interval map as more such stores are discovered. No additional walking or aliasing queries are added. If the IntervalMap forms an interval covering the the entire earlier store, then it is dead and can be removed.<br>
<br>
I discovered this problem when investigating a performance issue with code like this on PowerPC:<br>
<br>
  #include <complex><br>
  using namespace std;<br>
<br>
  complex<float> bar(complex<float> C);<br>
  complex<float> foo(complex<float> C) {<br>
    return bar(C)*C;<br>
  }<br>
<br>
which produces this:<br>
<br>
  define void @_Z4testSt7complexIfE(%"struct.std::complex"* noalias nocapture sret %agg.result, i64 %c.coerce) {<br>
  entry:<br>
    %ref.tmp = alloca i64, align 8<br>
    %tmpcast = bitcast i64* %ref.tmp to %"struct.std::complex"*<br>
    %c.sroa.0.0.extract.shift = lshr i64 %c.coerce, 32<br>
    %c.sroa.0.0.extract.trunc = trunc i64 %c.sroa.0.0.extract.shift to i32<br>
    %0 = bitcast i32 %c.sroa.0.0.extract.trunc to float<br>
    %c.sroa.2.0.extract.trunc = trunc i64 %c.coerce to i32<br>
    %1 = bitcast i32 %c.sroa.2.0.extract.trunc to float<br>
    call void @_Z3barSt7complexIfE(%"struct.std::complex"* nonnull sret %tmpcast, i64 %c.coerce)<br>
    %2 = bitcast %"struct.std::complex"* %agg.result to i64*<br>
    %3 = load i64, i64* %ref.tmp, align 8<br>
    store i64 %3, i64* %2, align 4 ; <--- ***** THIS SHOULD NOT BE HERE ****<br>
    %_M_value.realp.i.i = getelementptr inbounds %"struct.std::complex", %"struct.std::complex"* %agg.result, i64 0, i32 0, i32 0<br>
    %4 = lshr i64 %3, 32<br>
    %5 = trunc i64 %4 to i32<br>
    %6 = bitcast i32 %5 to float<br>
    %_M_value.imagp.i.i = getelementptr inbounds %"struct.std::complex", %"struct.std::complex"* %agg.result, i64 0, i32 0, i32 1<br>
    %7 = trunc i64 %3 to i32<br>
    %8 = bitcast i32 %7 to float<br>
    %mul_ad.i.i = fmul fast float %6, %1<br>
    %mul_bc.i.i = fmul fast float %8, %0<br>
    %mul_i.i.i = fadd fast float %mul_ad.i.i, %mul_bc.i.i<br>
    %mul_ac.i.i = fmul fast float %6, %0<br>
    %mul_bd.i.i = fmul fast float %8, %1<br>
    %mul_r.i.i = fsub fast float %mul_ac.i.i, %mul_bd.i.i<br>
    store float %mul_r.i.i, float* %_M_value.realp.i.i, align 4<br>
    store float %mul_i.i.i, float* %_M_value.imagp.i.i, align 4<br>
    ret void<br>
  }<br>
<br>
the problem here is not just that the i64 store is unnecessary, but also that it blocks further backend optimizations of the other uses of that i64 value in the backend.<br>
<br>
For the value of the interval map, I'm currently using std::tuple<> (because it is empty).<br>
<br>
<br>
<a href="http://reviews.llvm.org/D18586" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18586</a><br>
<br>
Files:<br>
  lib/Transforms/Scalar/DeadStoreElimination.cpp<br>
  test/Transforms/DeadStoreElimination/combined-partial-overwrites.ll<br>
<br>
</blockquote></div><br></div>