[PATCH] D94862: [InstCombine] Replace one-use select operand based on condition

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

I found the file that's getting miscompiled, and uploaded preprocessed source and some notes here: https://bugs.chromium.org/p/chromium/issues/detail?id=1167890#c6

We do have code that matches what this change is looking for:

  last_child_covers_ = child == nullptr ? previous_child : child;

And it seems that previous_child is getting folded to nullptr, which is incorrect. It's not clear whether it's this change in itself that's bad, or if it interacts badly with some underlying issue.

In D94862#2505484 <https://reviews.llvm.org/D94862#2505484>, @aqjune wrote:

> Hmm, would the pointer replacement be problematic here?
> If there is a reduced version of a code, I think we can try
>
>   -    if (isa<Constant>(CmpRHS) && !isa<ConstantExpr>(CmpRHS))
>   +    if (isa<Constant>(CmpRHS) && !isa<ConstantExpr>(CmpRHS) && CmpRHS->getType()->isIntOrIntVectorTy())
>
> and see whether the crash goes away.

Yes, it seems that it's a pointer that's being replaced, but I don't see why the value type should matter for this optimization.

I've reverted in 58bdfcfac048563e0dbcecc7c75e4e7897c8da18 <https://reviews.llvm.org/rG58bdfcfac048563e0dbcecc7c75e4e7897c8da18> until this can be figured out (that also reverted Tres's follow-up change).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94862