[PATCH] D98351: [llvm-opt] Bug fix within combining FP vectors

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1642
     return FPExt->getOperand(0)->getType();
 
   // If this value is a constant, return the constant in the smallest FP type
----------------
I think the reason it gets to `shrinkFPConstant` is because the above `dyn_cast<FPExtInst>` returns false, even though `V` is a `fpext` constant expression.

If you add:

  +  if (auto *FPCExt = dyn_cast<ConstantExpr>(V))
  +    if (FPCExt->getOpcode() == Instruction::FPExt)
  +      return FPCExt->getOperand(0)->getType();

Then your test will pass.

The problem is quite specific to scalable vector splat-operations which cannot constant fold into something simpler, thus leading to a ConstantExpression. I'm not entirely sure if we should duplicate such code for ConstantExprs. In this case it seems like a pretty trivial fix that isn't that much more complicated than bailing out for scalable vectors, so perhaps this is fine, but it does seem a bit arbitrary.

Personally I would be happy to go either way, either the proposed solution or bailing out and avoiding this improvement until we hit this as something that needs fixing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98351