[PATCH] D55988: WIP: RegBankSelect: Support some more complex part mappings

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked an inline comment as done.
arsenm added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h:629
+  /// be inserted.
+  virtual unsigned getBreakDownCost(const ValueMapping &ValMapping) const {
+    return std::numeric_limits<unsigned>::max();
----------------
qcolombet wrote:
> arsenm wrote:
> > qcolombet wrote:
> > > I feel we miss a register bank in that prototype (what you'd referenced as CurBank).
> > I had this initially, but then discovered it will always be null. The array of banks is present in ValMapping
> The banks we have in the value mapping are the one we want to use (destination), not the one the value originates from (source), right?
> 
> >  then discovered it will always be null. 
> 
> Are you just patching up the destination in your case (i.e., things that are not assigned to any regbank yet)?
> 
> I would have expected this information to be here and useful in cases like this:
> orig: dest = op src
> candidate mapping: dest.low = op.low src.low; dest.high = op.high src.high
> Thus src has a regbank at this point, but dest doesn't.
> 
> Although I am not a fan of such approach, we could take the convention for now that curbank == null means we are patching a definition and curbank != null means we are patching a source.
> E.g.,
> 1. dest = op => dest.low = op.low ; dest.high = op.high
> vs.
> 2. = op src => = op.low src.low ; = op.high src.high
> 
> In #1 dest's regbank would be nullptr, in #2 src's regbank would be !nullptr.
I based this off only seeing null, plus the above assert:

  assert((!IsSameNumOfValues || CurRegBank) && "We should not have to repair");



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

https://reviews.llvm.org/D55988