<div dir="ltr">sdiv also prevents converting divide by power of 2 into a shift. That's what was happening in the loop I was looking at. For x86 we ended up with a multiple instruction abs idiom, a shift for the divide, and then undoing the abs.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 29, 2016 at 12:20 PM, Huang, Li1 <span dir="ltr"><<a href="mailto:li1.huang@intel.com" target="_blank">li1.huang@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Actually, D18777 is doing the same thing as Craig's change, and is based on the same motivation: udiv/urem -> sdiv/srem. Udiv/urem is generally faster than sdiv/srem on most archs.<br>
<br>
The reason it breaks the FE test is that it converts some sexts to zexts before indvars (during instcombine), but indvars cannot recognize that the zext is unnecessary and should be removed. Instead, indvars inserts truncs before zexts after widening the IV.<br>
<br>
D18867 was proposed to fix this problem, this change teaches indvars that zext of a signed value which is known non-negative is unnecessary. However, this change itself is not enough, a loop-rotate pass is needed before indvars for this change to work.<br>
<br>
The test uses O1 but O1 pipeline doesn't have a loop-rotate before indvars, neither does O2 or O3. So I proposed to change the test in D21773.<br>
<br>
Thanks,<br>
Li Huang<br>
<div class="HOEnZb"><div class="h5"><br>
-----Original Message-----<br>
From: Sanjoy Das [mailto:<a href="mailto:sanjoy@playingwithpointers.com">sanjoy@playingwithpointers.com</a>]<br>
Sent: Wednesday, June 29, 2016 12:35 AM<br>
To: Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>><br>
Cc: Adam Nemet <<a href="mailto:anemet@apple.com">anemet@apple.com</a>>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>>; Huang, Li1 <<a href="mailto:li1.huang@intel.com">li1.huang@intel.com</a>><br>
Subject: Re: [llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.<br>
<br>
I think I have to redact my "I suspect this isn't anything complicated" tone. :)<br>
<br>
To do the right thing in cases like the one you pastebinned, we'll have to track `IsSigned` on individual `NarrowIVDefUse` instances.<br>
<br>
Then for instance, in<br>
<br>
   %i.050 = phi i32 [ %add15, %for.body ], [ 0, %for.body.preheader ]<br>
   %idxprom45 = zext i32 %i.050 to i64<br>
   %arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom45<br>
   %0 = load i32, i32* %arrayidx, align 4<br>
   %sub = add nsw i32 %i.050, -1<br>
   %idxprom1 = sext i32 %sub to i64<br>
   %arrayidx2 = getelementptr inbounds i32, i32* %B, i64 %idxprom1<br>
   %1 = load i32, i32* %arrayidx2, align 4<br>
<br>
we'll first replace `%idxprom45` with the zero extended wide IV, and then when we get to `%sub`, we'll replace it with `add %wide.zexted.iv, -1` but the corresponding `NarrowIVDefUse` we push to the worklist will have to remember that this new wide add instruction is really a `sext` of `%sub` (and accordingly handle its users).<br>
<br>
I'll be on vacation from 30th June to 10th of July, but I should be able to get to this once I'm back (unfortunately, I probably won't have time before I leave).  If you want to fix this and have someone else or me (once I'm back) review it that's fine too.<br>
<br>
Thanks,<br>
-- Sanjoy<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div>
</div>