This is an archive of the discontinued LLVM Phabricator instance.

[VE] Minimal codegen for empty functions
ClosedPublic

Authored by simoll on Jan 13 2020, 4:57 AM.

Details

Summary

This patch implements minimal VE code generation for empty function bodies (no args, no value return).

Contents

  • empty function code generation test.
  • Minimal function prologue & epilogue emission
  • Instruction formats and instruction definitions as far as required for the empty function prologue & epilogue.
  • I64 register class definitions.

This patch deviates from the common practice of having separate patches for the register file, instruction formats and instructions that offer self-contained logic but come without any tests. We are trying something new here: by implementing the (close to) minimum of everything in one patch, this patch is a minimal testable unit for VE code generation.

Event Timeline

simoll created this revision.Jan 13 2020, 4:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2020, 4:57 AM
simoll updated this revision to Diff 237631.Jan 13 2020, 5:08 AM

Stripped custom VEMachineFunction

arsenm added inline comments.Jan 13 2020, 6:12 AM
llvm/lib/Target/VE/InstPrinter/VEInstPrinter.cpp
61

Why bother with the truncation?

82–86

Invert condition and avoid empty true block

llvm/lib/Target/VE/VEFrameLowering.h
8

Drop empty comment section?

simoll updated this revision to Diff 237659.Jan 13 2020, 6:25 AM
simoll marked 2 inline comments as done.

Addressed arsenm's comments.

simoll marked an inline comment as done.Jan 13 2020, 6:26 AM

added a meaningful header description

arsenm accepted this revision.Jan 13 2020, 10:02 AM

LGTM with a few more nits

llvm/lib/Target/VE/VEFrameLowering.cpp
307

Capitalize comments and demorgan

llvm/lib/Target/VE/VEISelLowering.cpp
79

s/unsigned/Register

llvm/lib/Target/VE/VEInstrInfo.cpp
106–108

.add*s on separate line like other BuildMI calls? It was mixed in other files too

llvm/lib/Target/VE/VEMCInstLower.cpp
41

dead break

This revision is now accepted and ready to land.Jan 13 2020, 10:02 AM
simoll updated this revision to Diff 237922.Jan 14 2020, 4:01 AM
simoll marked 4 inline comments as done.
Changes
  • Argument additions to BuildMI on separate lines everywhere.
  • Fixed LO32/HI32 immediate truncation.
  • Stripped unused reg pressure set from VERegisterInfo.
  • Capitalized comments.
arsenm added inline comments.Jan 14 2020, 5:29 AM
llvm/lib/Target/VE/VE.h
97–99

MathExtras.h already has Lo_32/Hi_32 for this

simoll updated this revision to Diff 237965.Jan 14 2020, 7:10 AM

We use LO32, HI32 here to always represent immediates in the numerical domain of LLVM immediates (signed int64_t).
The advantage of this is that we can check whether any immediates are representable as signed 32bit literals when emitting assembly.
When using LLVM's Lo_32/Hi_32 along with other sources of immediates it is not possible to tell in the asm printer whether 0x00000000FFFFFFFF should be -1 or is in fact too large for signed 32bit integer literals.

arsenm added inline comments.Jan 14 2020, 7:41 AM
llvm/lib/Target/VE/InstPrinter/VEInstPrinter.cpp
64–66

assert(isInt<32>())

simoll updated this revision to Diff 237986.Jan 14 2020, 8:16 AM
simoll marked an inline comment as done.
  • Rebased
  • isInt<32>(MO.getImm())
This revision was automatically updated to reflect the committed changes.