[PATCH] D84399: [SCEVExpander] Avoid re-using existing casts if it means updating users.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 30 12:11:56 PDT 2020
fhahn added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:112
+ // If we skipped past MustDominate, use MustDominate as insertion point.
+ if (IP->getParent() == MustDominate->getParent() &&
+ !IP->comesBefore(MustDominate))
----------------
efriedma wrote:
> I'm not sure I understand what's happening here. "I" dominates "MustDominate" because the we're inserting an instruction that uses I. And we're assuming MustDominate itself must be a legal insertion point. So the only way we could skip past MustDominate is if IsInsertedInstruction() is true for MustDominate.
>
> Is it actually possible for isInsertedInstruction(MustDominate) to be true? If it is, could we check it directly?
> Is it actually possible for isInsertedInstruction(MustDominate) to be true? If it is, could we check it directly?
Unfortunately, at the moment there is a case where we might mark existing instructions as 'inserted' (
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp#L1247)
This check is indeed a workaround, in case such an instruction is an insert point, so checking directly for it would be more straight forward. I'll update the code, thanks!
================
Comment at: llvm/test/Transforms/LoopStrengthReduce/pr27056.ll:39
; CHECK: for.body:
; CHECK-NEXT: [[TMP5:%.*]] = inttoptr i64 [[LSR_IV]] to %struct.L*
-; CHECK-NEXT: [[CMP:%.*]] = icmp eq %struct.L* [[UGLYGEP1]], [[TMP]]
----------------
fhahn wrote:
> lebedev.ri wrote:
> > Why were we emitting an unused instruction in the first place?
> That's probably due to the cast re-using logic, which in some cases makes certain values dead by replacing all uses with an equivalent cast.
(which this patch removes)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84399/new/
https://reviews.llvm.org/D84399
More information about the llvm-commits
mailing list