[PATCH] D147041: [CodeGen] Remove redundent instructions generated by combineAddrModes.
Peter Rong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
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
More information about the llvm-commits
mailing list