Opened 2 years ago

Last modified 9 months ago

#13904 new bug

LLVM does not need to trash caller-saved registers.

Reported by: kavon Owned by:
Priority: normal Milestone: 8.10.1
Component: Compiler Version: 8.0.1
Keywords: llvm, codegen Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: #4992, #4308 Differential Rev(s): Phab:D5190
Wiki Page:

Description

Summary: This ticket is a simplification of the LLVM backend that I already plan to do in another branch, but I'd like to push the patch into master, and then merge from master into that branch.

The LLVM backend attempts to trash the physical registers corresponding to Rx, Fx, and Dx registers that are caller-saved before an FFI call by emitting a sequence such as this:

  store i64 undef, i64* %R3_Var
  store i64 undef, i64* %R4_Var
  store i64 undef, i64* %R5_Var
  store i64 undef, i64* %R6_Var
  store float undef, float* %F1_Var
  store double undef, double* %D1_Var
  store float undef, float* %F2_Var
  store double undef, double* %D2_Var
  store float undef, float* %F3_Var
  store double undef, double* %D3_Var
  store float undef, float* %F4_Var
  store double undef, double* %D4_Var
  store float undef, float* %F5_Var
  store double undef, double* %D5_Var
  store float undef, float* %F6_Var
  store double undef, double* %D6_Var
  %ln7bu = call ccc double (double) @llvm.sin.f64( double %ln7bs ) nounwind

Where the Rx/Fx/Dx registers are chosen based on what physical register those global registers map to, and whether those physical registers are caller-saved according to the C ABI.

I'm certain this is unnecessary, as there is no way to know the physical register corresponding to those vars within an LLVM IR function. The calling convention used for the FFI call will preserve caller-saved registers as needed, i.e., only if such a value is needed after the call.

Change History (14)

comment:1 Changed 2 years ago by kavon

Owner: set to kavon

comment:2 Changed 2 years ago by kavon

Summary: LLVM does not need to trash callee-saved registers.LLVM does not need to trash caller-saved registers.

comment:3 Changed 2 years ago by bgamari

Let us know when you have a patch, kavon.

comment:4 Changed 2 years ago by kavon

comment:5 Changed 2 years ago by kavon

Current patch is under the branch wip/T13904. Soon I will test/check the patch for the issues described in the related tickets to ensure there are no regressions.

comment:6 Changed 2 years ago by bgamari

Status: newpatch

comment:7 Changed 13 months ago by thomie

Type of failure: Compile-time performance bugRuntime performance bug

See Performance/Runtime for all runtime performance tickets.

comment:8 Changed 12 months ago by monoidal

Differential Rev(s): Phab:D5190

comment:9 Changed 12 months ago by Ben Gamari <ben@…>

In adcb5fb4/ghc:

Multiple fixes / improvements for LLVM backend

- Fix for #13904 -- stop "trashing" callee-saved registers, since it is
  not actually doing anything useful.

- Fix for #14251 -- fixes the calling convention for functions passing
  raw SSE-register values by adding padding as needed to get the values
  in the right registers. This problem cropped up when some args were
  unused an dropped from the live list.

- Fixed a typo in 'readnone' attribute

- Added 'lower-expect' pass to level 0 LLVM optimization passes to
  improve block layout in LLVM for stack checks, etc.

Test Plan: `make test WAYS=optllvm` and `make test WAYS=llvm`

Reviewers: bgamari, simonmar, angerman

Reviewed By: angerman

Subscribers: rwbarton, carter

GHC Trac Issues: #13904, #14251

Differential Revision: https://phabricator.haskell.org/D5190

comment:10 Changed 12 months ago by bgamari

Milestone: 8.6.1
Status: patchmerge

comment:11 Changed 12 months ago by bgamari

Milestone: 8.6.18.6.2

comment:12 Changed 11 months ago by bgamari

Resolution: fixed
Status: mergeclosed

comment:13 Changed 11 months ago by bgamari

Milestone: 8.6.28.8.1
Owner: kavon deleted
Resolution: fixed
Status: closednew

This was reverted in ghc-8.6 due to regressions.

comment:14 Changed 9 months ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

Note: See TracTickets for help on using tickets.