Page MenuHomePhabricator

[RISCV] Support Constant Pools in Load/Store Peephole
ClosedPublic

Authored by lenary on May 6 2020, 2:57 PM.

Details

Summary

RISC-V uses a post-select peephole pass to optimise
(load/store (ADDI $reg, %lo(addr)), 0) into (load/store $reg, %lo(addr)).
This peephole wasn't firing for accesses to constant pools, which is how we
materialise most floating point constants.

This adds support for the constantpool case, which improves code generation for
lots of small FP loading examples. I have not added any tests because this
structure is well-covered by the fp-imm.ll testcases, as well as almost
all other uses of floating point constants in the RISC-V backend tests.

Diff Detail

Unit TestsFailed

TimeTest
50 msLLVM.CodeGen/RISCV::Unknown Unit Message ("")
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/bin/llc -mtriple=riscv32 -verify-machineinstrs < /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/llvm/test/CodeGen/RISCV/select-const.ll | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/bin/FileCheck -check-prefix=RV32I /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/llvm/test/CodeGen/RISCV/select-const.ll

Event Timeline

lenary created this revision.May 6 2020, 2:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2020, 2:57 PM
luismarques accepted this revision.May 6 2020, 3:09 PM

LGTM. Nice to see this extraneous addi go!

This revision is now accepted and ready to land.May 6 2020, 3:09 PM
asb accepted this revision.May 6 2020, 10:17 PM

LGTM, thanks. Left an inline comment pointing out a TODO that can now be removed thanks to this.

llvm/test/CodeGen/RISCV/float-imm.ll
25

This TODO can be deleted thanks to this patch!

lenary updated this revision to Diff 262570.May 7 2020, 2:11 AM
  • Remove TODO which is now done!
lenary updated this revision to Diff 262653.May 7 2020, 7:54 AM
  • I missed a test on a previous update.
This revision was automatically updated to reflect the committed changes.
lenary reopened this revision.May 11 2020, 10:16 AM

I had to back this out in rG3242e5653a5 due to an API update which caused my build to fail. Will re-land soon.

This revision is now accepted and ready to land.May 11 2020, 10:16 AM
lenary updated this revision to Diff 263217.May 11 2020, 10:49 AM
  • getAlign API Update
This revision was automatically updated to reflect the committed changes.