[PATCH] D75014: [InstrEmitter, SystemZ] Copy Access registers with the correct register class.

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 25 07:38:59 PST 2020


uweigand added a comment.

I'm wondering if this handles all cases ... for CopyFromReg you apparently rely on the logic in EmitCopyFromReg that checks whether the value is used by some MachineNode with constrained regclass.  But that logic isn't unconditionally used, e.g. it is skipped for "cloned" SUs ... not sure whether this could cause issues in more complicated scenarios.

Also, I'm not sure if the Glue handling is fully correct: for CopyToReg, you seem to simply drop the glue, which doesn't look right.  For CopyFromReg, you keep the glue ... but you also glue the new node in as well, which may not be necessary (and may actually confuse code in EmitNode that scans glue chains?).

That said, if we do need to handle this problem in target code, it might actually be cleaner to just do it as a separate PreReload pass that simply looks at COPY nodes and constrains source/target virtual registers if required.  This could be something like the X86 FlagsCopyLowering pass, I guess (but much simpler).

(Talking about flags, it seems that COPY from/to the %cc register would also require the same handling as access registers.  Well, I guess we could implement a copy *to* %cc from a high register using TMHH, but we cannot implement a copy from %cc to a high register ...)


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

https://reviews.llvm.org/D75014





More information about the llvm-commits mailing list