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

[Zhang Kang via Phabricator via llvm-commits]

ZhangKang marked 2 inline comments as done.
ZhangKang 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--;
----------------
arsenm wrote:
> 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
I have removed calling `unionImplicitOps()`  in PPCEarlyReturn.cpp here, and enabled the patch D76042 to use `setDesc()` to fix the bug in PPCEarlyReturn.cpp as you suggested.
Now there is no scene to call `unionImplicitOps()`.
 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78010