[all-commits] [llvm/llvm-project] dcfe19: [MCP][NFC] Opinionated refactoring (#186239)

Scott Linder via All-commits all-commits at lists.llvm.org
Thu Apr 16 09:20:01 PDT 2026


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: dcfe195ef1cb7590207cc7a89765482cfb164767
      https://github.com/llvm/llvm-project/commit/dcfe195ef1cb7590207cc7a89765482cfb164767
  Author: Scott Linder <scott.linder at amd.com>
  Date:   2026-04-16 (Thu, 16 Apr 2026)

  Changed paths:
    M llvm/lib/CodeGen/MachineCopyPropagation.cpp

  Log Message:
  -----------
  [MCP][NFC] Opinionated refactoring (#186239)

There are a few minor inconsistencies across the pass which I found mildly distracting:

* The use of `Def`/`Dest`/`Dst` to refer to the same thing
* Inconsistent declaration order of `Dst`/`Src` vs `Src`/`Dst`
* Lots of `->getReg()->asMCReg()`, and uses of `Register` when the pass
  is always running after RA anyway.
* Some places explicitly `assert(isCopyInstr)` while others just deref
  the `optional`.

Standardize on `Dst`/`Src` to match the metaphor and ordering of
`DestSourcePair`.

Assume `std::optional::operator*` will assert in any reasonable
implementation, even though this may technically be undefined behavior.
When asserts are disabled it would be anyway.

The refactor uses structured bindings for a couple reasons:

* Naturally enforces consistent order of `Dst`-then-`Src`
* Requires the use of `auto`, which ensures the declaration is not
  implicitly converting from `MCRegister` back to `Register`.

In both cases the explicitness of the name `getDstSrcMCRegs` hopefully
makes the meaning at the callsite clear (`Dst, Src = DstSrc`, and
explicitly mentioning `MCReg`).

Change-Id: Ic58f555e03535d726cdad38dbe3f9c6df1b86460



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications


More information about the All-commits mailing list