[PATCH] D55872: RegBankSelect: Fix copy insertion point for terminators

Quentin Colombet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 20 16:34:18 PST 2018


qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

> I thought about this, but figured since in this case it's most likely for not really copyable condition registers it might end up being problematic

Good point. Also we have to potentially deal with physical registers (because of ABI constraints for instance) and I think the idea was to keep the live-ranges as small as possible when repairing.

Something that occurred to me is that originally I planned to declare an insertion point between two terminators as being a split point (using InsertionPoint::HasSplit). I gave up on the idea because

1. I didn't have a use case
2. The APIs for querying the terminators are actually clunky and don't allow to properly update the successors list.

See the comments in InstrInsertPoint::materialize if you're curious, but that would be the right way of doing IMHO.

Anyway, LGTM.

Cheers,
-Quentin


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

https://reviews.llvm.org/D55872





More information about the llvm-commits mailing list