[llvm] 53d238a - [CodeGen] Fixing inconsistent ABI mangling of vlaues in SelectionDAGBuilder

Lucas Prates via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 21 02:05:41 PDT 2020


Author: Lucas Prates
Date: 2020-09-21T10:05:34+01:00
New Revision: 53d238a961d14eae46f6f2b296ce48026c7bd0a1

URL: https://github.com/llvm/llvm-project/commit/53d238a961d14eae46f6f2b296ce48026c7bd0a1
DIFF: https://github.com/llvm/llvm-project/commit/53d238a961d14eae46f6f2b296ce48026c7bd0a1.diff

LOG: [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 #47454.

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D87844

Added: 
    llvm/test/CodeGen/ARM/pr47454.ll

Modified: 
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 530ede44548a..dcfa6e405c74 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -151,32 +151,6 @@ static cl::opt<unsigned> SwitchPeelThreshold(
 // store [4096 x i8] %data, [4096 x i8]* %buffer
 static const unsigned MaxParallelChains = 64;
 
-// Return the calling convention if the Value passed requires ABI mangling as it
-// is a parameter to a function or a return value from a function which is not
-// an intrinsic.
-static Optional<CallingConv::ID> getABIRegCopyCC(const Value *V) {
-  if (auto *R = dyn_cast<ReturnInst>(V))
-    return R->getParent()->getParent()->getCallingConv();
-
-  if (auto *CI = dyn_cast<CallInst>(V)) {
-    const bool IsInlineAsm = CI->isInlineAsm();
-    const bool IsIndirectFunctionCall =
-        !IsInlineAsm && !CI->getCalledFunction();
-
-    // It is possible that the call instruction is an inline asm statement or an
-    // indirect function call in which case the return value of
-    // getCalledFunction() would be nullptr.
-    const bool IsInstrinsicCall =
-        !IsInlineAsm && !IsIndirectFunctionCall &&
-        CI->getCalledFunction()->getIntrinsicID() != Intrinsic::not_intrinsic;
-
-    if (!IsInlineAsm && !IsInstrinsicCall)
-      return CI->getCallingConv();
-  }
-
-  return None;
-}
-
 static SDValue getCopyFromPartsVector(SelectionDAG &DAG, const SDLoc &DL,
                                       const SDValue *Parts, unsigned NumParts,
                                       MVT PartVT, EVT ValueVT, const Value *V,
@@ -1587,7 +1561,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) {
     unsigned InReg = FuncInfo.InitializeRegForValue(Inst);
 
     RegsForValue RFV(*DAG.getContext(), TLI, DAG.getDataLayout(), InReg,
-                     Inst->getType(), getABIRegCopyCC(V));
+                     Inst->getType(), None);
     SDValue Chain = DAG.getEntryNode();
     return RFV.getCopyFromRegs(DAG, FuncInfo, getCurSDLoc(), Chain, nullptr, V);
   }
@@ -5520,7 +5494,7 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
     if (VMI != FuncInfo.ValueMap.end()) {
       const auto &TLI = DAG.getTargetLoweringInfo();
       RegsForValue RFV(V->getContext(), TLI, DAG.getDataLayout(), VMI->second,
-                       V->getType(), getABIRegCopyCC(V));
+                       V->getType(), None);
       if (RFV.occupiesMultipleRegs()) {
         splitMultiRegDbgValue(RFV.getRegsAndSizes());
         return true;

diff  --git a/llvm/test/CodeGen/ARM/pr47454.ll b/llvm/test/CodeGen/ARM/pr47454.ll
new file mode 100644
index 000000000000..d36a29c4e77c
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/pr47454.ll
@@ -0,0 +1,49 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=armv8-unknown-linux-unknown -mattr=-fp16 -O0 < %s | FileCheck %s
+
+declare fastcc half @getConstant()
+
+declare fastcc i1 @isEqual(half %0, half %1)
+
+define internal fastcc void @main() {
+; CHECK-LABEL: main:
+; CHECK:       @ %bb.0: @ %Entry
+; CHECK-NEXT:    push {r11, lr}
+; CHECK-NEXT:    mov r11, sp
+; CHECK-NEXT:    sub sp, sp, #16
+; CHECK-NEXT:    mov r0, #31744
+; CHECK-NEXT:    strh r0, [r11, #-2]
+; CHECK-NEXT:    ldrh r0, [r11, #-2]
+; CHECK-NEXT:    bl __gnu_h2f_ieee
+; CHECK-NEXT:    vmov s0, r0
+; CHECK-NEXT:    vstr s0, [sp, #8] @ 4-byte Spill
+; CHECK-NEXT:    bl getConstant
+; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    bl __gnu_h2f_ieee
+; CHECK-NEXT:    vmov s0, r0
+; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    bl __gnu_f2h_ieee
+; CHECK-NEXT:    vldr s0, [sp, #8] @ 4-byte Reload
+; CHECK-NEXT:    vmov r1, s0
+; CHECK-NEXT:    str r0, [sp, #4] @ 4-byte Spill
+; CHECK-NEXT:    mov r0, r1
+; CHECK-NEXT:    bl __gnu_f2h_ieee
+; CHECK-NEXT:    uxth r0, r0
+; CHECK-NEXT:    vmov s0, r0
+; CHECK-NEXT:    ldr r0, [sp, #4] @ 4-byte Reload
+; CHECK-NEXT:    uxth r1, r0
+; CHECK-NEXT:    vmov s1, r1
+; CHECK-NEXT:    bl isEqual
+; CHECK-NEXT:    mov sp, r11
+; CHECK-NEXT:    pop {r11, pc}
+Entry:
+    ; First arg directly from constant
+    %const = alloca half, align 2
+    store half 0xH7C00, half* %const, align 2
+    %arg1 = load half, half* %const, align 2
+    ; Second arg from fucntion return
+    %arg2 = call fastcc half @getConstant()
+    ; Arguments should have equivalent mangling
+    %result = call fastcc i1 @isEqual(half %arg1, half %arg2)
+    ret void
+}


        


More information about the llvm-commits mailing list