aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Voung <jvoung@chromium.org>2013-08-30 16:42:36 -0700
committerJan Voung <jvoung@chromium.org>2013-08-30 16:42:36 -0700
commit121830a16cdb2685b1ac49bb88407644c044ef30 (patch)
tree1f8189150dc41b8b29609206cc12246afded44cc
parent06bf94d7c3fd693b3c6e3b71178514e5ed59489a (diff)
Revert some ARM byval localmods since byval+varargs are not in stable pexes.
Localmods came from: https://codereview.chromium.org/10825082/, and earlier. (1) The original change was so that byval parameters always go on the stack. That part was added because the original ARM code was buggy, and did not actually make a copy of the value, modifying the caller's struct (ouch!). (2) Then came a localmod to make all arguments following a byval go on the stack and to make the var-args code aware of that. This is so that arguments stay in the correct order for var-args to pick up. For (1) there has been some work upstream to make it work better. In any case, clang with --target=armv7a-...-gnueabi only used byval in some limited cases -- when the size of the struct is > 64 bytes where the backend will know that part of it could be in regs, and the rest can be memcpy'ed to the stack. For le32, clang will still generate byval without satisfying the same ARM condition (only for structs bigger than 64 bytes), so it could be *very bad* if we didn't have the ABI simpification passes rewrite the byval and try to let the ARM backend do things with byval... TEST=the GCC torture tests: va-arg-4.c, and 20030914-2.c and the example in issue 2746 still pass. BUG=none, cleanup R=dschuff@chromium.org Review URL: https://codereview.chromium.org/23691009
-rw-r--r--include/llvm/CodeGen/CallingConvLower.h14
-rw-r--r--lib/CodeGen/CallingConvLower.cpp1
-rw-r--r--lib/Target/ARM/ARMCallingConv.td4
-rw-r--r--lib/Target/ARM/ARMISelLowering.cpp32
-rw-r--r--test/NaCl/Localmods/arm-byval-ref-fix.ll33
5 files changed, 0 insertions, 84 deletions
diff --git a/include/llvm/CodeGen/CallingConvLower.h b/include/llvm/CodeGen/CallingConvLower.h
index 0b401f8f8e..fa9d60f0d4 100644
--- a/include/llvm/CodeGen/CallingConvLower.h
+++ b/include/llvm/CodeGen/CallingConvLower.h
@@ -213,7 +213,6 @@ private:
// InRegsParamsProceed - shows how many instances of ByValRegs was proceed
// during argument analysis.
unsigned InRegsParamsProceed;
- bool HasByValInRegPosition; // @LOCALMOD -- ARM only: see comment below.
protected:
ParmContext CallOrPrologue;
@@ -395,19 +394,6 @@ public:
ByValRegs.clear();
}
- // @LOCALMOD-BEGIN
- // We disabled the splitting of byval between registers and memory.
- // This separate flag indicates that a byval existed. We cannot reuse
- // isFirstByValRegValid() because that is already used by the broken
- // mechanism of splitting between stack and regs. We should check
- // again if this mechanism is still broken later, or try to fix that
- // mechanism.
- // NOTE: this is only for ARM, so should be refactored.
- bool hasByValInRegPosition() const { return HasByValInRegPosition; }
- void setHasByValInRegPosition() { HasByValInRegPosition = true; }
- void clearHasByValInRegPosition() { HasByValInRegPosition = false; }
- // @LOCALMOD-END
-
ParmContext getCallOrPrologue() const { return CallOrPrologue; }
private:
diff --git a/lib/CodeGen/CallingConvLower.cpp b/lib/CodeGen/CallingConvLower.cpp
index d4cc1a8654..75f4b96e3b 100644
--- a/lib/CodeGen/CallingConvLower.cpp
+++ b/lib/CodeGen/CallingConvLower.cpp
@@ -33,7 +33,6 @@ CCState::CCState(CallingConv::ID CC, bool isVarArg, MachineFunction &mf,
StackOffset = 0;
clearByValRegsInfo();
- clearHasByValInRegPosition(); // @LOCALMOD.
UsedRegs.resize((TRI.getNumRegs()+31)/32);
}
diff --git a/lib/Target/ARM/ARMCallingConv.td b/lib/Target/ARM/ARMCallingConv.td
index b051d88534..1a41448149 100644
--- a/lib/Target/ARM/ARMCallingConv.td
+++ b/lib/Target/ARM/ARMCallingConv.td
@@ -105,10 +105,6 @@ def CC_ARM_APCS_GHC : CallingConv<[
def CC_ARM_AAPCS_Common : CallingConv<[
- // @LOCALMOD-BEGIN (PR11018)
- CCIfByVal<CCPassByVal<4, 4>>,
- // @LOCALMOD-END
-
CCIfType<[i1, i8, i16], CCPromoteToType<i32>>,
// i64/f64 is passed in even pairs of GPRs
diff --git a/lib/Target/ARM/ARMISelLowering.cpp b/lib/Target/ARM/ARMISelLowering.cpp
index df7615df17..57c247901c 100644
--- a/lib/Target/ARM/ARMISelLowering.cpp
+++ b/lib/Target/ARM/ARMISelLowering.cpp
@@ -1795,30 +1795,6 @@ ARMTargetLowering::HandleByVal(
State->getCallOrPrologue() == Call) &&
"unhandled ParmContext");
- // @LOCALMOD-BEGIN
- // The original mechanism tries to split a byval argument between registers
- // and the stack. It doesn't work correctly yet, so disable it.
- // This leaves the entire byval argument on the stack, and the rest
- // of the parameters will need to be on the stack as well, to have
- // the correct order for var-args. We remember the fact that there was
- // a byval param that forced this, so that we know not to use the
- // handle var-args reg-save area.
- // PR11018.
- if (Subtarget->isTargetNaCl()) {
- unsigned ByValArgsCount = State->getInRegsParamsCount();
- unsigned CurByValIndex = State->getInRegsParamsProceed();
- if ((CurByValIndex >= ByValArgsCount) &&
- (ARM::R0 <= reg) && (reg <= ARM::R3)) {
- State->setHasByValInRegPosition();
- }
- // Confiscate any remaining parameter registers to preclude their
- // assignment to subsequent parameters.
- while (State->AllocateReg(GPRArgRegs, 4))
- ;
- return;
- }
- // @LOCALMOD-END
-
// For in-prologue parameters handling, we also introduce stack offset
// for byval registers: see CallingConvLower.cpp, CCState::HandleByVal.
// This behaviour outsides AAPCS rules (5.5 Parameters Passing) of how
@@ -2868,10 +2844,6 @@ ARMTargetLowering::computeRegArea(CCState &CCInfo, MachineFunction &MF,
unsigned RBegin, REnd;
CCInfo.getInRegsParamInfo(InRegsParamRecordIdx, RBegin, REnd);
NumGPRs = REnd - RBegin;
- // @LOCALMOD-BEGIN
- } else if (Subtarget->isTargetNaCl() && CCInfo.hasByValInRegPosition()) {
- NumGPRs = 0;
- // @LOCALMOD-END
} else {
unsigned int firstUnalloced;
firstUnalloced = CCInfo.getFirstUnallocated(GPRArgRegs,
@@ -2922,10 +2894,6 @@ ARMTargetLowering::StoreByValRegs(CCState &CCInfo, SelectionDAG &DAG,
CCInfo.getInRegsParamInfo(InRegsParamRecordIdx, RBegin, REnd);
firstRegToSaveIndex = RBegin - ARM::R0;
lastRegToSaveIndex = REnd - ARM::R0;
- // @LOCALMOD-BEGIN
- } else if (Subtarget->isTargetNaCl() && CCInfo.hasByValInRegPosition()) {
- firstRegToSaveIndex = 4; // Nothing to save.
- // @LOCALMOD-END
} else {
firstRegToSaveIndex = CCInfo.getFirstUnallocated
(GPRArgRegs, sizeof(GPRArgRegs) / sizeof(GPRArgRegs[0]));
diff --git a/test/NaCl/Localmods/arm-byval-ref-fix.ll b/test/NaCl/Localmods/arm-byval-ref-fix.ll
deleted file mode 100644
index 13759a7f6a..0000000000
--- a/test/NaCl/Localmods/arm-byval-ref-fix.ll
+++ /dev/null
@@ -1,33 +0,0 @@
-; RUN: pnacl-llc -march=arm -mtriple=armv7a-none-nacl %s -o - | FileCheck %s
-
-; byval is currently crashing on ARM for upstream LLVM (PR11018).
-; We have a LOCALMOD in ARMISelLowering to simply leave byval wholly
-; on the stack, so this is expected to pass.
-
-%struct.S = type { i32, i32 }
-
-define void @foo(%struct.S* byval %w) nounwind {
-entry:
-
-; Verify that 55 is stored onto the stack directly, so the struct is
-; passed by value and not by reference.
-
-; CHECK: foo:
-; CHECK-NEXT: entry
-; CHECK-NEXT: mov [[REG:r[0-9]+]], #55
-; CHECK-NEXT: str [[REG]], [sp]
-
- %x = getelementptr inbounds %struct.S* %w, i32 0, i32 0
- store i32 55, i32* %x, align 4
- ret void
-}
-
-define i32 @main() nounwind {
-entry:
- %w = alloca %struct.S, align 4
- store %struct.S { i32 0, i32 0 }, %struct.S* %w
- call void @foo(%struct.S* byval %w)
- %x = getelementptr inbounds %struct.S* %w, i32 0, i32 0
- %retval = load i32* %x, align 4
- ret i32 %retval
-}