Skip to content

Commit b513e19

Browse files
pratlucaszmodem
authored andcommitted
[CodeGen] Fixing inconsistent ABI mangling of vlaues in SelectionDAGBuilder
SelectionDAGBuilder was inconsistently mangling values based on ABI Calling Conventions when getting them through copyFromRegs in SelectionDAGBuilder, causing duplicate value type convertions for function arguments. The checking for the mangling requirement was based on the value's originating instruction and was performed outside of, and inspite of, the regular Calling Convention Lowering. The issue could be observed in a scenario such as: ``` %arg1 = load half, half* %const, align 2 %arg2 = call fastcc half @someFunc() call fastcc void @otherFunc(half %arg1, half %arg2) ; Here, %arg2 was incorrectly mangled twice, as the CallConv data from ; the call to @someFunc() was taken into consideration for the check ; when getting the value for processing the call to @otherFunc(...), ; after the proper convertion had taken place when lowering the return ; value of the first call. ``` This patch fixes the issue by disregarding the Calling Convention information for such copyFromRegs, making sure the ABI mangling is properly contanined in the Calling Convention Lowering. This fixes Bugzilla llvm#47454. Reviewed By: efriedma Differential Revision: https://reviews.llvm.org/D87844 (cherry picked from commit 53d238a)
1 parent 6250d49 commit b513e19

File tree

2 files changed

+51
-28
lines changed

2 files changed

+51
-28
lines changed

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -169,32 +169,6 @@ static cl::opt<unsigned> SwitchPeelThreshold(
169169
// store [4096 x i8] %data, [4096 x i8]* %buffer
170170
static const unsigned MaxParallelChains = 64;
171171

172-
// Return the calling convention if the Value passed requires ABI mangling as it
173-
// is a parameter to a function or a return value from a function which is not
174-
// an intrinsic.
175-
static Optional<CallingConv::ID> getABIRegCopyCC(const Value *V) {
176-
if (auto *R = dyn_cast<ReturnInst>(V))
177-
return R->getParent()->getParent()->getCallingConv();
178-
179-
if (auto *CI = dyn_cast<CallInst>(V)) {
180-
const bool IsInlineAsm = CI->isInlineAsm();
181-
const bool IsIndirectFunctionCall =
182-
!IsInlineAsm && !CI->getCalledFunction();
183-
184-
// It is possible that the call instruction is an inline asm statement or an
185-
// indirect function call in which case the return value of
186-
// getCalledFunction() would be nullptr.
187-
const bool IsInstrinsicCall =
188-
!IsInlineAsm && !IsIndirectFunctionCall &&
189-
CI->getCalledFunction()->getIntrinsicID() != Intrinsic::not_intrinsic;
190-
191-
if (!IsInlineAsm && !IsInstrinsicCall)
192-
return CI->getCallingConv();
193-
}
194-
195-
return None;
196-
}
197-
198172
static SDValue getCopyFromPartsVector(SelectionDAG &DAG, const SDLoc &DL,
199173
const SDValue *Parts, unsigned NumParts,
200174
MVT PartVT, EVT ValueVT, const Value *V,
@@ -1624,7 +1598,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) {
16241598
unsigned InReg = FuncInfo.InitializeRegForValue(Inst);
16251599

16261600
RegsForValue RFV(*DAG.getContext(), TLI, DAG.getDataLayout(), InReg,
1627-
Inst->getType(), getABIRegCopyCC(V));
1601+
Inst->getType(), None);
16281602
SDValue Chain = DAG.getEntryNode();
16291603
return RFV.getCopyFromRegs(DAG, FuncInfo, getCurSDLoc(), Chain, nullptr, V);
16301604
}
@@ -5555,7 +5529,7 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
55555529
if (VMI != FuncInfo.ValueMap.end()) {
55565530
const auto &TLI = DAG.getTargetLoweringInfo();
55575531
RegsForValue RFV(V->getContext(), TLI, DAG.getDataLayout(), VMI->second,
5558-
V->getType(), getABIRegCopyCC(V));
5532+
V->getType(), None);
55595533
if (RFV.occupiesMultipleRegs()) {
55605534
splitMultiRegDbgValue(RFV.getRegsAndSizes());
55615535
return true;

llvm/test/CodeGen/ARM/pr47454.ll

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
2+
; RUN: llc -mtriple=armv8-unknown-linux-unknown -mattr=-fp16 -O0 < %s | FileCheck %s
3+
4+
declare fastcc half @getConstant()
5+
6+
declare fastcc i1 @isEqual(half %0, half %1)
7+
8+
define internal fastcc void @main() {
9+
; CHECK-LABEL: main:
10+
; CHECK: @ %bb.0: @ %Entry
11+
; CHECK-NEXT: push {r11, lr}
12+
; CHECK-NEXT: mov r11, sp
13+
; CHECK-NEXT: sub sp, sp, #16
14+
; CHECK-NEXT: mov r0, #31744
15+
; CHECK-NEXT: strh r0, [r11, #-2]
16+
; CHECK-NEXT: ldrh r0, [r11, #-2]
17+
; CHECK-NEXT: bl __gnu_h2f_ieee
18+
; CHECK-NEXT: vmov s0, r0
19+
; CHECK-NEXT: vstr s0, [sp, #8] @ 4-byte Spill
20+
; CHECK-NEXT: bl getConstant
21+
; CHECK-NEXT: vmov r0, s0
22+
; CHECK-NEXT: bl __gnu_h2f_ieee
23+
; CHECK-NEXT: vmov s0, r0
24+
; CHECK-NEXT: vmov r0, s0
25+
; CHECK-NEXT: bl __gnu_f2h_ieee
26+
; CHECK-NEXT: vldr s0, [sp, #8] @ 4-byte Reload
27+
; CHECK-NEXT: vmov r1, s0
28+
; CHECK-NEXT: str r0, [sp, #4] @ 4-byte Spill
29+
; CHECK-NEXT: mov r0, r1
30+
; CHECK-NEXT: bl __gnu_f2h_ieee
31+
; CHECK-NEXT: uxth r0, r0
32+
; CHECK-NEXT: vmov s0, r0
33+
; CHECK-NEXT: ldr r0, [sp, #4] @ 4-byte Reload
34+
; CHECK-NEXT: uxth r1, r0
35+
; CHECK-NEXT: vmov s1, r1
36+
; CHECK-NEXT: bl isEqual
37+
; CHECK-NEXT: mov sp, r11
38+
; CHECK-NEXT: pop {r11, pc}
39+
Entry:
40+
; First arg directly from constant
41+
%const = alloca half, align 2
42+
store half 0xH7C00, half* %const, align 2
43+
%arg1 = load half, half* %const, align 2
44+
; Second arg from fucntion return
45+
%arg2 = call fastcc half @getConstant()
46+
; Arguments should have equivalent mangling
47+
%result = call fastcc i1 @isEqual(half %arg1, half %arg2)
48+
ret void
49+
}

0 commit comments

Comments
 (0)