[PATCH] D153838: [MCP] Optimize copies from undef

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 28 06:07:35 PDT 2023


foad added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:486-487
+  if (!CopyOperands->Source->isUndef()) {
+    PrevCopy->getOperand(PrevCopyOperands->Source->getOperandNo())
+        .setIsUndef(false);
+  }
----------------
Pierre-vh wrote:
> Pierre-vh wrote:
> > arsenm wrote:
> > > Pierre-vh wrote:
> > > > foad wrote:
> > > > > Doesn't `PrevCopyOperands->Source->setIsUndef(false)` work?
> > > > The reference to the MachineOperand is const, unfortunately
> > > You can change that?
> > It's `struct DestSourcePair`, it's sometimes initialized from `const MachineInstr` so I can't just remove the const qualifier there.
> > 
> > An alternative may be to create a copy of that struct and name it `MutableDestSourcePair`, and use it where relevant. 
> In particular, the issue is with `isCopyInstr` which is used to generate the value here. It takes a const instruction and many user also pass const instructions, so it'd be a bit of a refactor (not sure how big).
> 
> If we really want to avoid the code above, then I will need to create an overload of that function that takes a non-const instr and returns a `MutableDestSourcePair`.
> 
> IMO, the piece of code above is ok and not worth the refactor, but I'd also like to get this in soon as it's a blocker for several tickets so I have no strong opinions - I just need to know how to proceed :)
I think the code above is fine as-is, now that you've explained the reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153838



More information about the llvm-commits mailing list