[PATCH] D78010: [CodeGen] Add new function unionImplicitOps() to union implicit register

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCEarlyReturn.cpp:80-81
               // branch with a blr.
               BuildMI(**PI, J, J->getDebugLoc(), TII->get(I->getOpcode()))
-                  .copyImplicitOps(*I);
+                  .unionImplicitOps(*I);
               MachineBasicBlock::iterator K = J--;
----------------
ZhangKang wrote:
> arsenm wrote:
> > Do you just want to setDesc here instead? It seems to me you expect the same implicit operands before and after in the test, and just want to change the opcode
> In fact, I have another patch https://reviews.llvm.org/D76042 to use `setDesc()`. I have set the patch D76042 be `Change Planed`.  
> In fact, if we use `setDesc()`, it means that we have assumed that the old instruction's implicit operands are same with the new instruction's.
> 
> If this patch can be approved, I will abandoned the patch D76042. I think use `unionImplicitOps `  is better， it doesn't assume that the old instruction's implicit operands are same with the new instruction's.
There's no reason to not rely on this assumption though. I can imagine a use for unionImplicitOps, but not for contexts such as this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78010