[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Here is the bugpoint-reduced test case for the SPEC failure when enabling -msave-restore and allowing tail calls:

Run the command llc test.ll -mattr=+save-restore -o out.s

You will see the code generated is wrong:

  tail    __riscv_restore_2
   jr      a5

target datalayout = "e-m:e-p:32:32-i64:64-n32-S128"
target triple = "riscv32-unknown-linux-gnu"

%struct.1_s = type { i8*, i8*, i8*}
%struct.2_s = type { i8*, i8*, i8*}

declare dso_local void @test2() local_unnamed_addr
declare dso_local i32 @test3(%struct.2_s* nonnull %a, %struct.1_s** %b) local_unnamed_addr

define dso_local void @test1(%struct.2_s* %a, %struct.1_s** %b) local_unnamed_addr  {
entry:

  br i1 undef, label %if.end2, label %if.then

if.then:                                          ; preds = %entry

  tail call void @test2()
  br label %if.end2

if.end2:                                          ; preds = %if.then, %entry

  %call3 = tail call i32 null(%struct.2_s* nonnull %a, %struct.1_s** %b)
  ret void

}

To summarize the issues we have detected so far:

1. Bug1: When transforming a tail call back into a regular call, the transformation is not correctly preserving the original call's return values.

I fixed this by conservatively adding RegState::Implicit definitions for each possible integer and FP return registers, when defining the riscv_restore tail call.

2. Bug2: test case provided above. Please Lewis take a look at how this case can be fixed.

3. I have also run the tests with "-fno-optimize-sibling-calls -msave-restore", i.e., disabling tail calls when m-save-restore is enabled.

With this config, issues (1) and (2) do not happen.
There is some loss in code size savings when using fno-optimize-sibling-calls.
But still we see code size savings benefit from -msave-restore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686