[PATCH] D33186: [InstCombine] Canonicalize clamp of float types to minmax in fast mode.

[Andrei Elovikov via Phabricator via llvm-commits]

a.elovikov added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:1326
+      bool IsCastNeeded = LHS->getType()->getPrimitiveSizeInBits() !=
+                              SelType->getPrimitiveSizeInBits();
+      Value *CmpLHS = cast<CmpInst>(CondVal)->getOperand(0);
----------------
a.elovikov wrote:
> efriedma wrote:
> > This is very suspicious.
> > 
> > The way the implementation of matchSelectPattern is written, CastOp is uninitialized if the type of the compare matches the type of the select; otherwise, it's set to whatever cast we looked through.  That cast might not be a cast which changes the size of the type; it could bit a BitCast/FPToUI/etc.
> > 
> > I'd like to see a few testcases which cover the situations where we insert casts.
> Yes, this exactly the case causing UB - passing uninitialized  object by value to createCast Corresponding argument is not even used in that function because of the early return when the types are equal.
> 
> I believe the case with cast should be already covered by earlier tests as I did not touch that part - will try to find them tomorrow, or add new ones if they're missing.
One more note - neither Aslan nor MSan catches this, only UBSan because the value of the uninitialized argument to createCast is not used.


https://reviews.llvm.org/D33186