[PATCH] D78010: [CodeGen] Add a new parameter SkipDuplicated for copyImplicitOps()

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

I think that the question that needs to be answered is whether the duplicated implicit operands were intended or if they were just an oversight. If they were intended, we should understand that intention and see if it still makes sense and if anything relies on it. If it was just an oversight, we should see if anything relies on it or whether we are free to change the semantics.
In any case, if we do need to keep the existing semantics, I would much rather have separate functions to achieve the different semantics. Perhaps `copyImplicitOperands()` should remain as-is and we should add something like `unionImplicitOperands()` that would create the union that you describe here.



================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:1425
+    if ((MO.isReg() && MO.isImplicit()) || MO.isRegMask()) {
+      if (SkipDuplicated && (findRegisterUseOperandIdx(MO.getReg()) != -1 ||
+                             findRegisterDefOperandIdx(MO.getReg()) != -1))
----------------
Won't this throw assertions on regmasks? And what should the behaviour actually be on multiple regmasks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78010