[PATCH] D54956: [ValueTracking] Look through casts when determining non-nullness

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:2024
+    // Look through various operations and instructions which do not alter the
+    // value, or at least not the nullness property of the value (sext/zext).
+    if (const BitCastOperator *BCO = dyn_cast<BitCastOperator>(V))
----------------
spatel wrote:
> jdoerfert wrote:
> > fhahn wrote:
> > > The dyn_casts are not strictly necessary I think and using a switch on the opcode might be a little bit more compact, together with getOperand(0). But it doesn't really make a big difference.
> > The switch is not much nicer, I think. Especially since I don't know how to add the operators, e.g., constant bitcast expressions, in the switch. Does anybody feel strongly about this?
> Seems ok to me, but if we're going to dyn_cast, you can use 'auto *' to make it less wordy:
> http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> 
> Another option to shrink the code would be something like this:
>   Value *X;
>   if (match(II, m_CombineOr(m_ZExtOrSExt(m_Value(X)), m_BitCast(m_Value(X)))))
>     return isKnownNonZero(X, Depth, Q);
> 
> We're missing an m_IntToPtr() matcher...if we added that, all of the cases could use matchers.
I can do `auto *` and I can try to implement it in a switch but I don't feel qualified to use the matcher logic just now. We also would need to compare "sizes" for int2ptr/ptr2int.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54956/new/

https://reviews.llvm.org/D54956