[PATCH] D103865: [SystemZ] Generate XC loop for memset 0 of variable length.

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

In D103865#2851145 <https://reviews.llvm.org/D103865#2851145>, @jonpa wrote:

> Updated per review.
>
>> I was thinking of just using emitInt16 for the three words, that will show as .word in the textual assembler output, which may not be pretty but is certainly correct. (You could in addition use an emitRawComment to print a textual representation to make the file more readable).
>
> Ah, ok, I did not realize at the time I could just print the instruction as numbers... I tried this instead now, since we don't really want to have a generic subtarget around. Is there any reason to print 3 x Int16?  Would it be better to print it as one (or three) hex if possible?

Well, there is no emitInt48 ...  You could use one emitInt16 and one emitInt32, but that's not really better I think.

I must admin that manual encoding code is longer and uglier than I had initially thought ...

>> In the meantime I thought of yet another option: the instructions used via EXRL are really part of the function containing the EXRL, and should therefore be emitted using the same Subtarget that is in effect for that function (this is not *currently* a problem, but it might be a potential issue if we want to use an EXRL target instruction that is only available in some ISA levels).
>
> This seems like the logical solution, but it is a bit unfortunate to have to go through all of that trouble just for this case where it is actually not really needed at all.

I think it wouldn't be all that much effort.  You should simply change EXRLT2SymMap to be a map from a **pair** of (const SystemZSubtarget *, MCInst) to MCSymbol *, add the current subtarget pointer to the key when processing SystemZ::EXRL_Pseudo in the AsmPrinter, and in emitEXRLTargetInstructions just iterate over the map (as now) -- you'll get the pairs of (const SystemZSubtarget *, MCInst) back as key, and simply emit that instruction using that subtarget.

Otherwise, this is now looking good to me.


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

https://reviews.llvm.org/D103865