[PATCH] D23468: [ValueTracking] Sign bit of shl cannot be both known one and known zero. Fix PR28926

[Li Huang via llvm-commits]

lihuang added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:1078
@@ -1079,1 +1077,3 @@
                                       KOF);
+    // If this shift has "nsw" keyword, then the result is either a poison 
+    // value or has the same sign bit as the first operand. 
----------------
sanjoy wrote:
> Can you do the same thing by a "normalization" step like:
> 
> ```
>   // Bits that are known to be "both" zero and one are arbitrarily selected to be zero.
>   KnownZero |= (KnownZero & KnownOne);
>   KnownOne &= ~KnownZero;
> ```
> 
> ?
> 
> If not, now that you have this logic here, can we clear out the logic
> from `KZF` and `KOF`?
> 
> Also, were you able to pinpoint _why_ we ended up with a bit both proved
> to be `0` and `1`?
We can't arbitrarily select 0 if they are both known to be zero, as this will hide some cases where the shift result could be actually inferred as having the same sign (negative) as its first operand. One example is the last test I added in known-signbit-shift.ll.

Sure, I should have done that, my mistake, 

This is because we could have already shifted a KnownOne bit to the sign-bit by KOF when we are setting the inferred sign-bit of KnownZero in KZF, and also the other way around. For example:
                 before shift:    KnownZero = 0x70000000
                                        KnownOne =  0x40000000
               after SHL by 1:  KnownZero = 0x00000000   inferred to 0x70000000
                                         KnownOne = 0x70000000 (conflict)
 
We should keep the sign-bit of KnownZero and KnownOne to be the same as the first operand of shift. Actually this inspired me to fix the problem just in KZF and KOF. Anyway, I will make another patch after you revert r278172.



https://reviews.llvm.org/D23468