[llvm] r339350 - [MC] Remove PhysRegSize from MCRegisterClass

[Justin Bogner via llvm-commits]

Bjorn Pettersson via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: bjope
> Date: Thu Aug  9 08:19:07 2018
> New Revision: 339350
>
> URL: http://llvm.org/viewvc/llvm-project?rev=339350&view=rev
> Log:
> [MC] Remove PhysRegSize from MCRegisterClass
>
> Summary:
> The interface to get size and spill size of a register
> was moved from MCRegisterInfo to TargetRegisterInfo over
> a year ago. Afaik the old interface has been around
> to give out-of-tree targets a chance to adapt to the
> new interface.

When you make a change like this, please actually point out what the old
and new interface are, and how to upgrade from one to the other. My out
of tree backend is currently broken and it's very unclear how I'm
supposed to change my code to continue to work instead of calling
MCRegisterInfo::getSize, based on this commit message.

My point is, "It was moved over a year ago" isn't very easy to go on,
and it's generally better to say something like it was moved in rXYZ,
and clients should now call foo() instead. I realize it's a bit of a
burden, but the more details you can provide on how to upgrade the
better, and folks with out of tree targets really appreciate it if you
can put that extra little bit of effort in.

Thanks.

> One problem with the old MCRegisterClass::PhysRegSize was that
> it represented the size of a register as "size in bits" / 8.
> So a register had to be a multiple of eight bits wide for the
> size to be correct (and the byte size for the target needed to
> be eight bits).
>
> Reviewers: kparzysz, qcolombet
>
> Reviewed By: kparzysz
>
> Subscribers: llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D47199
>
> Modified:
>     llvm/trunk/include/llvm/MC/MCRegisterInfo.h
>     llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
>     llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
>
> Modified: llvm/trunk/include/llvm/MC/MCRegisterInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCRegisterInfo.h?rev=339350&r1=339349&r2=339350&view=diff
> ==============================================================================
>
> --- llvm/trunk/include/llvm/MC/MCRegisterInfo.h (original)
> +++ llvm/trunk/include/llvm/MC/MCRegisterInfo.h Thu Aug  9 08:19:07 2018
> @@ -41,7 +41,6 @@ public:
>    const uint16_t RegsSize;
>    const uint16_t RegSetSize;
>    const uint16_t ID;
> -  const uint16_t PhysRegSize;
>    const int8_t CopyCost;
>    const bool Allocatable;
>
> @@ -80,11 +79,6 @@ public:
>      return contains(Reg1) && contains(Reg2);
>    }
>
> -  /// Return the size of the physical register in bytes.
> -  unsigned getPhysRegSize() const { return PhysRegSize; }
> -  /// Temporary function to allow out-of-tree targets to switch.
> -  unsigned getSize() const { return getPhysRegSize(); }
> -
>    /// getCopyCost - Return the cost of copying a value between two registers in
>    /// this class. A negative number means the register class is very expensive
>    /// to copy e.g. status flag register classes.
>
> Modified: llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=339350&r1=339349&r2=339350&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp Thu Aug  9 08:19:07 2018
> @@ -388,8 +388,7 @@ bool TargetInstrInfo::getStackSlotRange(
>      return true;
>    }
>    unsigned BitSize = TRI->getSubRegIdxSize(SubIdx);
> -  // Convert bit size to byte size to be consistent with
> -  // MCRegisterClass::getSize().
> +  // Convert bit size to byte size.
>    if (BitSize % 8)
>      return false;
>
>
> Modified: llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp?rev=339350&r1=339349&r2=339350&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp Thu Aug  9 08:19:07 2018
> @@ -1075,14 +1075,10 @@ RegisterInfoEmitter::runMCDesc(raw_ostre
>
>    for (const auto &RC : RegisterClasses) {
>      assert(isInt<8>(RC.CopyCost) && "Copy cost too large.");
> -    uint32_t RegSize = 0;
> -    if (RC.RSI.isSimple())
> -      RegSize = RC.RSI.getSimple().RegSize;
>      OS << "  { " << RC.getName() << ", " << RC.getName() << "Bits, "
>         << RegClassStrings.get(RC.getName()) << ", "
>         << RC.getOrder().size() << ", sizeof(" << RC.getName() << "Bits), "
>         << RC.getQualifiedName() + "RegClassID" << ", "
> -       << RegSize/8 << ", "
>         << RC.CopyCost << ", "
>         << ( RC.Allocatable ? "true" : "false" ) << " },\n";
>    }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits