[llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.

[Huang, Li1 via llvm-commits]

Actually, this issue can be solved using ValueTracking's isKnownNonNegative when pushing "NarrowIVDefUse" to worklist, see my last comment in D18867. My change in D18867 used isKnownNonNegative initially, but later I tried to improve SCEV's isKnownPredicate to make it work for this case, however, it turned out to be related to the "nsw" flag propagation of SCEV, which is complicated and sort of controversial so I think I'd better not touch. 

I tried Sanjoy's suggestion, but it's hard to track that a "zext" is changed from a "sext" because the change is in InstCombine. 

Thanks,
Li Huang


-----Original Message-----
From: Huang, Li1 
Sent: Wednesday, July 20, 2016 4:49 PM
To: 'Sanjoy Das' <sanjoy at playingwithpointers.com>; Craig Topper <craig.topper at gmail.com>
Cc: Adam Nemet <anemet at apple.com>; llvm-commits <llvm-commits at lists.llvm.org>
Subject: RE: [llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.

Hi,

Is anyone already working on a fix for this issue? If not then I can work on this.


Thanks,
Li Huang


-----Original Message-----
From: Sanjoy Das [mailto:sanjoy at playingwithpointers.com] 
Sent: Wednesday, June 29, 2016 12:35 AM
To: Craig Topper <craig.topper at gmail.com>
Cc: Adam Nemet <anemet at apple.com>; llvm-commits <llvm-commits at lists.llvm.org>; Huang, Li1 <li1.huang at intel.com>
Subject: Re: [llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.

I think I have to redact my "I suspect this isn't anything complicated" tone. :)

To do the right thing in cases like the one you pastebinned, we'll have to track `IsSigned` on individual `NarrowIVDefUse` instances.

Then for instance, in

   %i.050 = phi i32 [ %add15, %for.body ], [ 0, %for.body.preheader ]
   %idxprom45 = zext i32 %i.050 to i64
   %arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom45
   %0 = load i32, i32* %arrayidx, align 4
   %sub = add nsw i32 %i.050, -1
   %idxprom1 = sext i32 %sub to i64
   %arrayidx2 = getelementptr inbounds i32, i32* %B, i64 %idxprom1
   %1 = load i32, i32* %arrayidx2, align 4

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).

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.

Thanks,
-- Sanjoy