Skip to content

Commit faac9f2

Browse files
committed
Revert "[X86][ABI] Don't preserve return regs for preserve_all/preserve_most CCs"
This caused Chromium to crash, see comment on the code review. > Currently both calling conventions preserve registers that are used to > store a return value. This causes the returned value to be lost: > > define i32 @bar() { > %1 = call preserve_mostcc i32 @foo() > ret i32 %1 > } > > define preserve_mostcc i32 @foo() { > ret i32 2 > ; preserve_mostcc will restore %rax, > ; whatever it was before the call. > } > > This contradicts the current documentation (preserve_allcc "behaves > identical to the `C` calling conventions on how arguments and return > values are passed") and also breaks [[clang::preserve_most]]. > > This change makes CSRs be preserved iff they are not used to store a > return value (e.g. %rax for scalars, {%rax:%rdx} for __int128, %xmm0 > for double). For void functions no additional registers are > preserved, i.e. the behaviour is backward compatible with existing > code. > > Differential Revision: https://reviews.llvm.org/D141020 This reverts commit 0276fa8.
1 parent 3821391 commit faac9f2

File tree

7 files changed

+135
-334
lines changed

7 files changed

+135
-334
lines changed

llvm/docs/LangRef.rst

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,8 @@ added in the future:
366366
apply for values returned in callee-saved registers.
367367

368368
- On X86-64 the callee preserves all general purpose registers, except for
369-
R11 and return registers, if any. R11 can be used as a scratch register.
370-
Floating-point registers (XMMs/YMMs) are not preserved and need to be
371-
saved by the caller.
369+
R11. R11 can be used as a scratch register. Floating-point registers
370+
(XMMs/YMMs) are not preserved and need to be saved by the caller.
372371

373372
The idea behind this convention is to support calls to runtime functions
374373
that have a hot path and a cold path. The hot path is usually a small piece

llvm/lib/Target/X86/X86CallingConv.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,11 +1154,11 @@ def CSR_64_CXX_TLS_Darwin_PE : CalleeSavedRegs<(add RBP)>;
11541154
// CSRs that are handled explicitly via copies.
11551155
def CSR_64_CXX_TLS_Darwin_ViaCopy : CalleeSavedRegs<(sub CSR_64_TLS_Darwin, RBP)>;
11561156

1157-
// All GPRs - except r11 and return registers.
1157+
// All GPRs - except r11
11581158
def CSR_64_RT_MostRegs : CalleeSavedRegs<(add CSR_64, RAX, RCX, RDX, RSI, RDI,
11591159
R8, R9, R10)>;
11601160

1161-
// All registers - except r11 and return registers.
1161+
// All registers - except r11
11621162
def CSR_64_RT_AllRegs : CalleeSavedRegs<(add CSR_64_RT_MostRegs,
11631163
(sequence "XMM%u", 0, 15))>;
11641164
def CSR_64_RT_AllRegs_AVX : CalleeSavedRegs<(add CSR_64_RT_MostRegs,

llvm/lib/Target/X86/X86ISelLowering.cpp

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -104,20 +104,6 @@ static void errorUnsupported(SelectionDAG &DAG, const SDLoc &dl,
104104
DiagnosticInfoUnsupported(MF.getFunction(), Msg, dl.getDebugLoc()));
105105
}
106106

107-
/// Returns true if a CC can dynamically exclude a register from the list of
108-
/// callee-saved-registers (TargetRegistryInfo::getCalleeSavedRegs()) based on
109-
/// params/returns.
110-
static bool shouldDisableCalleeSavedRegisterCC(CallingConv::ID CC) {
111-
switch (CC) {
112-
default:
113-
return false;
114-
case CallingConv::X86_RegCall:
115-
case CallingConv::PreserveMost:
116-
case CallingConv::PreserveAll:
117-
return true;
118-
}
119-
}
120-
121107
X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
122108
const X86Subtarget &STI)
123109
: TargetLowering(TM), Subtarget(STI) {
@@ -3188,7 +3174,7 @@ X86TargetLowering::LowerReturn(SDValue Chain, CallingConv::ID CallConv,
31883174
// In some cases we need to disable registers from the default CSR list.
31893175
// For example, when they are used for argument passing.
31903176
bool ShouldDisableCalleeSavedRegister =
3191-
shouldDisableCalleeSavedRegisterCC(CallConv) ||
3177+
CallConv == CallingConv::X86_RegCall ||
31923178
MF.getFunction().hasFnAttribute("no_caller_saved_registers");
31933179

31943180
if (CallConv == CallingConv::X86_INTR && !Outs.empty())
@@ -4340,7 +4326,7 @@ SDValue X86TargetLowering::LowerFormalArguments(
43404326
}
43414327
}
43424328

4343-
if (shouldDisableCalleeSavedRegisterCC(CallConv) ||
4329+
if (CallConv == CallingConv::X86_RegCall ||
43444330
F.hasFnAttribute("no_caller_saved_registers")) {
43454331
MachineRegisterInfo &MRI = MF.getRegInfo();
43464332
for (std::pair<Register, Register> Pair : MRI.liveins())
@@ -4901,7 +4887,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
49014887

49024888
// In some calling conventions we need to remove the used physical registers
49034889
// from the reg mask.
4904-
if (shouldDisableCalleeSavedRegisterCC(CallConv) || HasNCSR) {
4890+
if (CallConv == CallingConv::X86_RegCall || HasNCSR) {
49054891
const TargetRegisterInfo *TRI = Subtarget.getRegisterInfo();
49064892

49074893
// Allocate a new Reg Mask and copy Mask.

llvm/test/CodeGen/X86/preserve_allcc64-ret-double.ll

Lines changed: 0 additions & 54 deletions
This file was deleted.
Lines changed: 73 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -1,138 +1,82 @@
1-
; RUN: sed -e "s/RETTYPE/void/;s/RETVAL//" %s | llc -mtriple=x86_64-apple-darwin -mcpu=corei7 | FileCheck --check-prefixes=ALL,SSE,VOID %s
2-
; RUN: sed -e "s/RETTYPE/i32/;s/RETVAL/undef/" %s | llc -mtriple=x86_64-apple-darwin -mcpu=corei7 | FileCheck --check-prefixes=ALL,SSE,INT %s
3-
; RUN: sed -e "s/RETTYPE/\{i64\,i64\}/;s/RETVAL/undef/" %s | llc -mtriple=x86_64-apple-darwin -mcpu=corei7 | FileCheck --check-prefixes=ALL,SSE,INT128 %s
4-
;
5-
; RUN: sed -e "s/RETTYPE/void/;s/RETVAL//" %s | llc -mtriple=x86_64-apple-darwin -mcpu=corei7-avx | FileCheck --check-prefixes=ALL,AVX,VOID %s
6-
; RUN: sed -e "s/RETTYPE/i32/;s/RETVAL/undef/" %s | llc -mtriple=x86_64-apple-darwin -mcpu=corei7-avx | FileCheck --check-prefixes=ALL,AVX,INT %s
7-
; RUN: sed -e "s/RETTYPE/\{i64\,i64\}/;s/RETVAL/undef/" %s | llc -mtriple=x86_64-apple-darwin -mcpu=corei7-avx | FileCheck --check-prefixes=ALL,AVX,INT128 %s
1+
; RUN: llc < %s -mtriple=x86_64-apple-darwin -mcpu=corei7 | FileCheck --check-prefix=SSE %s
2+
; RUN: llc < %s -mtriple=x86_64-apple-darwin -mcpu=corei7-avx | FileCheck --check-prefix=AVX %s
83

9-
define preserve_allcc RETTYPE @preserve_allcc1() nounwind {
4+
define preserve_allcc void @preserve_allcc1() nounwind {
105
entry:
11-
;ALL-LABEL: preserve_allcc1
12-
;ALL: pushq %r10
13-
;ALL-NEXT: pushq %r9
14-
;ALL-NEXT: pushq %r8
15-
;ALL-NEXT: pushq %rdi
16-
;ALL-NEXT: pushq %rsi
17-
;VOID-NEXT: pushq %rdx
18-
;INT-NEXT: pushq %rdx
19-
;INT128-NOT: pushq %rdx
20-
;ALL-NEXT: pushq %rcx
21-
;VOID-NEXT: pushq %rax
22-
;INT-NOT: pushq %rax
23-
;INT128-NOT: pushq %rax
24-
;ALL-NEXT: pushq %rbp
25-
;ALL-NEXT: pushq %r15
26-
;ALL-NEXT: pushq %r14
27-
;ALL-NEXT: pushq %r13
28-
;ALL-NEXT: pushq %r12
29-
;ALL-NEXT: pushq %rbx
30-
;SSE: movaps %xmm15
31-
;SSE-NEXT: movaps %xmm14
32-
;SSE-NEXT: movaps %xmm13
33-
;SSE-NEXT: movaps %xmm12
34-
;SSE-NEXT: movaps %xmm11
35-
;SSE-NEXT: movaps %xmm10
36-
;SSE-NEXT: movaps %xmm9
37-
;SSE-NEXT: movaps %xmm8
38-
;SSE-NEXT: movaps %xmm7
39-
;SSE-NEXT: movaps %xmm6
40-
;SSE-NEXT: movaps %xmm5
41-
;SSE-NEXT: movaps %xmm4
42-
;SSE-NEXT: movaps %xmm3
43-
;SSE-NEXT: movaps %xmm2
44-
;SSE-NEXT: movaps %xmm1
45-
;SSE-NEXT: movaps %xmm0
46-
;AVX: vmovups %ymm15
47-
;AVX-NEXT: vmovups %ymm14
48-
;AVX-NEXT: vmovups %ymm13
49-
;AVX-NEXT: vmovups %ymm12
50-
;AVX-NEXT: vmovups %ymm11
51-
;AVX-NEXT: vmovups %ymm10
52-
;AVX-NEXT: vmovups %ymm9
53-
;AVX-NEXT: vmovups %ymm8
54-
;AVX-NEXT: vmovups %ymm7
55-
;AVX-NEXT: vmovups %ymm6
56-
;AVX-NEXT: vmovups %ymm5
57-
;AVX-NEXT: vmovups %ymm4
58-
;AVX-NEXT: vmovups %ymm3
59-
;AVX-NEXT: vmovups %ymm2
60-
;AVX-NEXT: vmovups %ymm1
61-
;AVX-NEXT: vmovups %ymm0
62-
;SSE: movaps {{.*}} %xmm0
63-
;SSE-NEXT: movaps {{.*}} %xmm1
64-
;SSE-NEXT: movaps {{.*}} %xmm2
65-
;SSE-NEXT: movaps {{.*}} %xmm3
66-
;SSE-NEXT: movaps {{.*}} %xmm4
67-
;SSE-NEXT: movaps {{.*}} %xmm5
68-
;SSE-NEXT: movaps {{.*}} %xmm6
69-
;SSE-NEXT: movaps {{.*}} %xmm7
70-
;SSE-NEXT: movaps {{.*}} %xmm8
71-
;SSE-NEXT: movaps {{.*}} %xmm9
72-
;SSE-NEXT: movaps {{.*}} %xmm10
73-
;SSE-NEXT: movaps {{.*}} %xmm11
74-
;SSE-NEXT: movaps {{.*}} %xmm12
75-
;SSE-NEXT: movaps {{.*}} %xmm13
76-
;SSE-NEXT: movaps {{.*}} %xmm14
77-
;SSE-NEXT: movaps {{.*}} %xmm15
78-
;AVX: vmovups {{.*}} %ymm0
79-
;AVX-NEXT: vmovups {{.*}} %ymm1
80-
;AVX-NEXT: vmovups {{.*}} %ymm2
81-
;AVX-NEXT: vmovups {{.*}} %ymm3
82-
;AVX-NEXT: vmovups {{.*}} %ymm4
83-
;AVX-NEXT: vmovups {{.*}} %ymm5
84-
;AVX-NEXT: vmovups {{.*}} %ymm6
85-
;AVX-NEXT: vmovups {{.*}} %ymm7
86-
;AVX-NEXT: vmovups {{.*}} %ymm8
87-
;AVX-NEXT: vmovups {{.*}} %ymm9
88-
;AVX-NEXT: vmovups {{.*}} %ymm10
89-
;AVX-NEXT: vmovups {{.*}} %ymm11
90-
;AVX-NEXT: vmovups {{.*}} %ymm12
91-
;AVX-NEXT: vmovups {{.*}} %ymm13
92-
;AVX-NEXT: vmovups {{.*}} %ymm14
93-
;AVX-NEXT: vmovups {{.*}} %ymm15
94-
;ALL: popq %rbx
95-
;ALL-NEXT: popq %r12
96-
;ALL-NEXT: popq %r13
97-
;ALL-NEXT: popq %r14
98-
;ALL-NEXT: popq %r15
99-
;ALL-NEXT: popq %rbp
100-
;VOID-NEXT: popq %rax
101-
;INT-NOT: popq %rax
102-
;INT128-NOT: popq %rax
103-
;ALL-NEXT: popq %rcx
104-
;VOID-NEXT: popq %rdx
105-
;INT-NEXT: popq %rdx
106-
;INT128-NOT: popq %rdx
107-
;ALL-NEXT: popq %rsi
108-
;ALL-NEXT: popq %rdi
109-
;ALL-NEXT: popq %r8
110-
;ALL-NEXT: popq %r9
111-
;ALL-NEXT: popq %r10
6+
;SSE-LABEL: preserve_allcc1
7+
;SSE: pushq %r10
8+
;SSE-NEXT: pushq %r9
9+
;SSE-NEXT: pushq %r8
10+
;SSE-NEXT: pushq %rdi
11+
;SSE-NEXT: pushq %rsi
12+
;SSE-NEXT: pushq %rdx
13+
;SSE-NEXT: pushq %rcx
14+
;SSE-NEXT: pushq %rax
15+
;SSE-NEXT: pushq %rbp
16+
;SSE-NEXT: pushq %r15
17+
;SSE-NEXT: pushq %r14
18+
;SSE-NEXT: pushq %r13
19+
;SSE-NEXT: pushq %r12
20+
;SSE-NEXT: pushq %rbx
21+
;SSE: movaps %xmm15
22+
;SSE-NEXT: movaps %xmm14
23+
;SSE-NEXT: movaps %xmm13
24+
;SSE-NEXT: movaps %xmm12
25+
;SSE-NEXT: movaps %xmm11
26+
;SSE-NEXT: movaps %xmm10
27+
;SSE-NEXT: movaps %xmm9
28+
;SSE-NEXT: movaps %xmm8
29+
;SSE-NEXT: movaps %xmm7
30+
;SSE-NEXT: movaps %xmm6
31+
;SSE-NEXT: movaps %xmm5
32+
;SSE-NEXT: movaps %xmm4
33+
;SSE-NEXT: movaps %xmm3
34+
;SSE-NEXT: movaps %xmm2
35+
;SSE-NEXT: movaps %xmm1
36+
;SSE-NEXT: movaps %xmm0
37+
;AVX-LABEL: preserve_allcc1
38+
;AVX: pushq %r10
39+
;AVX-NEXT: pushq %r9
40+
;AVX-NEXT: pushq %r8
41+
;AVX-NEXT: pushq %rdi
42+
;AVX-NEXT: pushq %rsi
43+
;AVX-NEXT: pushq %rdx
44+
;AVX-NEXT: pushq %rcx
45+
;AVX-NEXT: pushq %rax
46+
;AVX-NEXT: pushq %rbp
47+
;AVX-NEXT: pushq %r15
48+
;AVX-NEXT: pushq %r14
49+
;AVX-NEXT: pushq %r13
50+
;AVX-NEXT: pushq %r12
51+
;AVX-NEXT: pushq %rbx
52+
;AVX: vmovups %ymm15
53+
;AVX-NEXT: vmovups %ymm14
54+
;AVX-NEXT: vmovups %ymm13
55+
;AVX-NEXT: vmovups %ymm12
56+
;AVX-NEXT: vmovups %ymm11
57+
;AVX-NEXT: vmovups %ymm10
58+
;AVX-NEXT: vmovups %ymm9
59+
;AVX-NEXT: vmovups %ymm8
60+
;AVX-NEXT: vmovups %ymm7
61+
;AVX-NEXT: vmovups %ymm6
62+
;AVX-NEXT: vmovups %ymm5
63+
;AVX-NEXT: vmovups %ymm4
64+
;AVX-NEXT: vmovups %ymm3
65+
;AVX-NEXT: vmovups %ymm2
66+
;AVX-NEXT: vmovups %ymm1
67+
;AVX-NEXT: vmovups %ymm0
11268
call void asm sideeffect "", "~{rax},~{rbx},~{rcx},~{rdx},~{rsi},~{rdi},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{rbp},~{xmm0},~{xmm1},~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{xmm7},~{xmm8},~{xmm9},~{xmm10},~{xmm11},~{xmm12},~{xmm13},~{xmm14},~{xmm15}"()
113-
ret RETTYPE RETVAL
69+
ret void
11470
}
11571

116-
; Make sure R11 and return registers are saved before the call
117-
declare preserve_allcc RETTYPE @bar(i64, i64, double, double)
72+
; Make sure only R11 is saved before the call
73+
declare preserve_allcc void @bar(i64, i64, double, double)
11874
define void @preserve_allcc2() nounwind {
11975
entry:
120-
;ALL-LABEL: preserve_allcc2
121-
;VOID-NOT: movq %rax, [[REG1:%[a-z0-9]+]]
122-
;INT: movq %rax, [[REG1:%[a-z0-9]+]]
123-
;INT128: movq %rax, [[REG1:%[a-z0-9]+]]
124-
;VOID-NOT: movq %rdx, [[REG2:%[a-z0-9]+]]
125-
;INT-NOT: movq %rdx, [[REG2:%[a-z0-9]+]]
126-
;INT128: movq %rdx, [[REG2:%[a-z0-9]+]]
127-
;ALL: movq %r11, [[REG3:%[a-z0-9]+]]
128-
;ALL-NOT: movaps %xmm
129-
;VOID-NOT: movq {{.*}}, %rax
130-
;INT: movq [[REG1]], %rax
131-
;INT128: movq [[REG1]], %rax
132-
;VOID-NOT: movq {{.*}}, %rdx
133-
;INT-NOT: movq {{.*}}, %rdx
134-
;INT128: movq [[REG2]], %rdx
135-
;ALL: movq [[REG3]], %r11
76+
;SSE-LABEL: preserve_allcc2
77+
;SSE: movq %r11, [[REG:%[a-z0-9]+]]
78+
;SSE-NOT: movaps %xmm
79+
;SSE: movq [[REG]], %r11
13680
%a0 = call i64 asm sideeffect "", "={rax}"() nounwind
13781
%a1 = call i64 asm sideeffect "", "={rcx}"() nounwind
13882
%a2 = call i64 asm sideeffect "", "={rdx}"() nounwind
@@ -154,7 +98,7 @@ entry:
15498
%a21 = call <2 x double> asm sideeffect "", "={xmm13}"() nounwind
15599
%a22 = call <2 x double> asm sideeffect "", "={xmm14}"() nounwind
156100
%a23 = call <2 x double> asm sideeffect "", "={xmm15}"() nounwind
157-
call preserve_allcc RETTYPE @bar(i64 1, i64 2, double 3.0, double 4.0)
101+
call preserve_allcc void @bar(i64 1, i64 2, double 3.0, double 4.0)
158102
call void asm sideeffect "", "{rax},{rcx},{rdx},{r8},{r9},{r10},{r11},{xmm2},{xmm3},{xmm4},{xmm5},{xmm6},{xmm7},{xmm8},{xmm9},{xmm10},{xmm11},{xmm12},{xmm13},{xmm14},{xmm15}"(i64 %a0, i64 %a1, i64 %a2, i64 %a3, i64 %a4, i64 %a5, i64 %a6, <2 x double> %a10, <2 x double> %a11, <2 x double> %a12, <2 x double> %a13, <2 x double> %a14, <2 x double> %a15, <2 x double> %a16, <2 x double> %a17, <2 x double> %a18, <2 x double> %a19, <2 x double> %a20, <2 x double> %a21, <2 x double> %a22, <2 x double> %a23)
159103
ret void
160104
}

llvm/test/CodeGen/X86/preserve_mostcc64-ret-double.ll

Lines changed: 0 additions & 39 deletions
This file was deleted.

0 commit comments

Comments
 (0)