[PATCH] D147041: [CodeGen] Remove redundent instructions generated by combineAddrModes.

Wed Mar 29 04:54:31 PDT 2023

Peter added a comment.

My apology. I am not an expert in these piece of code, so some places are confusing to me as well. If you have a better way to fix the issue, I am more than happy to follow.

> I'm a bit confused. In the description you talk about new instructions introduced in AddressingModeCombiner::InsertPlaceholders.
> The patch itself does not do anything with then. So the patch deals with the value which is used in combined addressing mode.

The dead instruction is introduced in `findCommon`, which returns that instruction as `CommonValue`.

> Another question, why did you decide to erase this instruction only one place instead of erasing it in all early bail-outs?
> Don't you want to add this erasing into destructor of AddressingModeCombiner?

I think we can erase such dead instruction on all early bail-outs, but I couldn't find any test cases where leaving them there will cause problems (like hangs we had in the issue).

> why we continue sinking icmp while we have already have on in that BB from the bug?

Because `AddressingModeCombiner::InsertPlaceholders` keeps inserting placeholders (A select instruction) that happens to satisfy `sinkCmpExpression`'s conditions

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147041