[PATCH] D95703: [GlobalISel] Propagate extends through G_PHIs into the incoming value blocks.
Amara Emerson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 29 18:10:47 PST 2021
aemerson added inline comments.
================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3589
+
+ // If the target is likely to fold this extend away, don't propagate.
+ if (Builder.getTII().isExtendLikelyToBeFolded(*ExtMI, MRI))
----------------
arsenm wrote:
> Folded into what?
Could be anything. The idea is that extensions which are "free" because they'd be folded into another operation, are not worth propagating.
================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3597
+ SmallPtrSet <MachineInstr*, 4> InSrcs;
+ for (unsigned Idx = 1; Idx < MI.getNumOperands(); Idx += 2)
+ InSrcs.insert(getDefIgnoringCopies(MI.getOperand(Idx).getReg(), MRI));
----------------
paquette wrote:
> This can have at most 3 elements before returning false, so I guess you might as well cut the loop short here.
Unless I'm misunderstanding, this would prevent us from knowing how many unique incoming values there are, if we stop early. G_PHIs can have any number of operands.
================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3605
+
+ for (auto *DefMI : InSrcs) {
+ switch (DefMI->getOpcode()) {
----------------
paquette wrote:
> This can probably just go in the loop above, which should allow you to bail out early as well.
Sure.
================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3632
+ SetVector<MachineInstr *> SrcMIs;
+ DenseMap<MachineInstr *, MachineInstr *> OldToNewSrcMap;
+ for (unsigned SrcIdx = 1; SrcIdx < MI.getNumOperands(); SrcIdx += 2) {
----------------
paquette wrote:
> Why not `SmallDenseMap`?
Yeah, will change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95703/new/
https://reviews.llvm.org/D95703
More information about the llvm-commits
mailing list