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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 355400.
jonpa marked an inline comment as done.
jonpa added a comment.

> I think readability might be even better if you continue to create the MBBs in order, i.e. emit the EXRL at the end instead of at the beginning. (That means a duplicated LengthMO.isReg() test, but that should still be better.)

Makes sense... fixed.

> Ah, if you do it that way, you don't actually have to have multiple labels for the same instruction. Just compute the instruction (or just its opcode, see discussion in the main comment) first, and look it up in the table. If found, simply return its (one) associated symbol; if not, create a new symbol and add the pair of symbol, instruction to the table.
>
> Actually, that table can then just be a std::map (from instruction to symbol) instead of a vector, this implements the lookup in a more efficient way ...

ah, of course - only one label emitted now. They by this get sorted per the operands in the comparator for the std::map object, but a little detail then is that the labels now get emitted in any order (non-sorted) - but I suppose that's ok, right?

> There are ways around that. We do have a TargetMachine in the AsmPrinter, and there used to be a function getSubtargetImpl() that returns a "default" subtarget for the compilation unit. This has been deprecated (for the above reasons), but could probably be re-activated, and then be used as the subtarget to pass to emitInstruction during emitEndOfAsmFile.
>
> Another option would be to simply "assemble" those instructions by hand, i.e. compute the 6-byte integer value from the constituent fields.

I started out with trying to "assemble" by hand per your suggestion, but it seemed like quite some work, especially considering that we also have to be able to print it as text. So I tried instead with adding back the GenericSubtarget, which was simple enough.

This then eliminates another 3786 XC targets: there are no duplicates in any file, and at most I see 13 different XC targets in a single file :-)

SPEC  master <> patch

  cgije          :               115009               132379   +17370
  xc             :                13334                23198    +9864
  exrl           :                    0                 8873    +8873
  brctg          :                 8356                17228    +8872
  srlg           :                54407                63229    +8822
  brasl          :               640463               631658    -8805
  ...


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

https://reviews.llvm.org/D103865

Files:
  llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
  llvm/lib/Target/SystemZ/SystemZAsmPrinter.h
  llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
  llvm/lib/Target/SystemZ/SystemZInstrFormats.td
  llvm/lib/Target/SystemZ/SystemZInstrInfo.td
  llvm/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp
  llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp
  llvm/lib/Target/SystemZ/SystemZTargetMachine.h
  llvm/test/CodeGen/SystemZ/memset-05.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103865.355400.patch
Type: text/x-patch
Size: 24250 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210629/e3cfe278/attachment.bin>