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

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet 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();
----------------
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.


================
Comment at: lib/CodeGen/GlobalISel/RegBankSelect.cpp:209
+  assert(RepairPt.getNumInsertPoints() == 1 &&
+         "is it possible to have multiple?");
+
----------------
arsenm wrote:
> qcolombet wrote:
> > Yes, if we repair a physical register. In practice I've never seen that happen.
> An example would help, since handling this is what makes the more general handling more annoying
Phabricator being great, I don't actually see what this comment was referring too.
I believe it has to do with several insertion point and the answer is I don't have any example either. We could construct fake ones, but I avoided that so far to not be in a situation where we have live code just because our crazy mind can produce such case. I.e., I would rather have something realistic or ensure those never happen with some machine verifier checks.

Having a report_fatal_error would be fine as we figure out a way to build such real example.


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

https://reviews.llvm.org/D55988