[llvm] 6b76d82 - GlobalISel: Fix marking byval arguments as immutable
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 12 06:02:05 PST 2021
Author: Matt Arsenault
Date: 2021-03-12T09:01:53-05:00
New Revision: 6b76d82853fdc64160dda44610407c04af620292
URL: https://github.com/llvm/llvm-project/commit/6b76d82853fdc64160dda44610407c04af620292
DIFF: https://github.com/llvm/llvm-project/commit/6b76d82853fdc64160dda44610407c04af620292.diff
LOG: GlobalISel: Fix marking byval arguments as immutable
byval arguments need to be assumed writable. Only implicitly stack
passed arguments which aren't addressable in the IR can be assumed
immutable.
Mips is still broken since for some reason its doing its own thing
with the ValueHandlers (and x86 doesn't actually handle byval
arguments now, although some of the code is there).
Added:
llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-stack-objects.ll
Modified:
llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
llvm/lib/Target/ARM/ARMCallLowering.cpp
llvm/lib/Target/Mips/MipsCallLowering.cpp
llvm/lib/Target/X86/X86CallLowering.cpp
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-function-args.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h b/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
index 0d35e424c3ce..0d587c14a393 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
@@ -149,7 +149,8 @@ class CallLowering {
/// should be initialized to an appropriate description of the
/// address created.
virtual Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) = 0;
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) = 0;
/// The specified value has been assigned to a physical register,
/// handle the appropriate COPY (either to or from) and mark any
diff --git a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
index 946950db9383..f689801fa30f 100644
--- a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
@@ -639,7 +639,8 @@ bool CallLowering::handleAssignments(CCState &CCInfo,
: LocVT.getStoreSize();
unsigned Offset = VA.getLocMemOffset();
MachinePointerInfo MPO;
- Register StackAddr = Handler.getStackAddress(MemSize, Offset, MPO);
+ Register StackAddr =
+ Handler.getStackAddress(MemSize, Offset, MPO, Flags);
Handler.assignValueToAddress(Args[i], Part, StackAddr, MemSize, MPO,
VA);
continue;
@@ -652,8 +653,8 @@ bool CallLowering::handleAssignments(CCState &CCInfo,
continue;
MachinePointerInfo MPO;
- Register StackAddr = Handler.getStackAddress(Flags.getByValSize(),
- VA.getLocMemOffset(), MPO);
+ Register StackAddr = Handler.getStackAddress(
+ Flags.getByValSize(), VA.getLocMemOffset(), MPO, Flags);
assert(Args[i].Regs.size() == 1 &&
"didn't expect split byval pointer");
MIRBuilder.buildCopy(Args[i].Regs[0], StackAddr);
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
index ce1ec905a862..d16e2fd13475 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -58,9 +58,15 @@ struct IncomingArgHandler : public CallLowering::IncomingValueHandler {
: IncomingValueHandler(MIRBuilder, MRI, AssignFn), StackUsed(0) {}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
auto &MFI = MIRBuilder.getMF().getFrameInfo();
- int FI = MFI.CreateFixedObject(Size, Offset, true);
+
+ // Byval is assumed to be writable memory, but other stack passed arguments
+ // are not.
+ const bool IsImmutable = !Flags.isByVal();
+
+ int FI = MFI.CreateFixedObject(Size, Offset, IsImmutable);
MPO = MachinePointerInfo::getFixedStack(MIRBuilder.getMF(), FI);
auto AddrReg = MIRBuilder.buildFrameIndex(LLT::pointer(0, 64), FI);
StackUsed = std::max(StackUsed, Size + Offset);
@@ -165,12 +171,15 @@ struct OutgoingArgHandler : public CallLowering::OutgoingValueHandler {
StackSize(0), SPReg(0) {}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
MachineFunction &MF = MIRBuilder.getMF();
LLT p0 = LLT::pointer(0, 64);
LLT s64 = LLT::scalar(64);
if (IsTailCall) {
+ assert(!Flags.isByVal() && "byval unhandled with tail calls");
+
Offset += FPDiff;
int FI = MF.getFrameInfo().CreateFixedObject(Size, Offset, true);
auto FIReg = MIRBuilder.buildFrameIndex(p0, FI);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp b/llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
index c0571e47f28e..e19677840f53 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
@@ -49,7 +49,8 @@ struct AMDGPUOutgoingValueHandler : public CallLowering::OutgoingValueHandler {
MachineInstrBuilder MIB;
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
llvm_unreachable("not implemented");
}
@@ -95,9 +96,14 @@ struct AMDGPUIncomingArgHandler : public CallLowering::IncomingValueHandler {
: IncomingValueHandler(B, MRI, AssignFn) {}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
auto &MFI = MIRBuilder.getMF().getFrameInfo();
- int FI = MFI.CreateFixedObject(Size, Offset, true);
+
+ // Byval is assumed to be writable memory, but other stack passed arguments
+ // are not.
+ const bool IsImmutable = !Flags.isByVal();
+ int FI = MFI.CreateFixedObject(Size, Offset, IsImmutable);
MPO = MachinePointerInfo::getFixedStack(MIRBuilder.getMF(), FI);
auto AddrReg = MIRBuilder.buildFrameIndex(
LLT::pointer(AMDGPUAS::PRIVATE_ADDRESS, 32), FI);
@@ -189,7 +195,8 @@ struct AMDGPUOutgoingArgHandler : public AMDGPUOutgoingValueHandler {
}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
MachineFunction &MF = MIRBuilder.getMF();
const LLT PtrTy = LLT::pointer(AMDGPUAS::PRIVATE_ADDRESS, 32);
const LLT S32 = LLT::scalar(32);
diff --git a/llvm/lib/Target/ARM/ARMCallLowering.cpp b/llvm/lib/Target/ARM/ARMCallLowering.cpp
index d4b920a43e89..fe8552cbeb44 100644
--- a/llvm/lib/Target/ARM/ARMCallLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMCallLowering.cpp
@@ -92,7 +92,8 @@ struct ARMOutgoingValueHandler : public CallLowering::OutgoingValueHandler {
: OutgoingValueHandler(MIRBuilder, MRI, AssignFn), MIB(MIB) {}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
assert((Size == 1 || Size == 2 || Size == 4 || Size == 8) &&
"Unsupported size");
@@ -244,13 +245,18 @@ struct ARMIncomingValueHandler : public CallLowering::IncomingValueHandler {
: IncomingValueHandler(MIRBuilder, MRI, AssignFn) {}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
assert((Size == 1 || Size == 2 || Size == 4 || Size == 8) &&
"Unsupported size");
auto &MFI = MIRBuilder.getMF().getFrameInfo();
- int FI = MFI.CreateFixedObject(Size, Offset, true);
+ // Byval is assumed to be writable memory, but other stack passed arguments
+ // are not.
+ const bool IsImmutable = !Flags.isByVal();
+
+ int FI = MFI.CreateFixedObject(Size, Offset, IsImmutable);
MPO = MachinePointerInfo::getFixedStack(MIRBuilder.getMF(), FI);
return MIRBuilder.buildFrameIndex(LLT::pointer(MPO.getAddrSpace(), 32), FI)
diff --git a/llvm/lib/Target/Mips/MipsCallLowering.cpp b/llvm/lib/Target/Mips/MipsCallLowering.cpp
index d5251ce10bd2..9cd6f7f9e151 100644
--- a/llvm/lib/Target/Mips/MipsCallLowering.cpp
+++ b/llvm/lib/Target/Mips/MipsCallLowering.cpp
@@ -175,6 +175,7 @@ Register MipsIncomingValueHandler::getStackAddress(const CCValAssign &VA,
unsigned Offset = VA.getLocMemOffset();
MachineFrameInfo &MFI = MF.getFrameInfo();
+ // FIXME: This should only be immutable for non-byval memory arguments.
int FI = MFI.CreateFixedObject(Size, Offset, true);
MachinePointerInfo MPO =
MachinePointerInfo::getFixedStack(MIRBuilder.getMF(), FI);
diff --git a/llvm/lib/Target/X86/X86CallLowering.cpp b/llvm/lib/Target/X86/X86CallLowering.cpp
index 28bba84c7448..bebc654ae741 100644
--- a/llvm/lib/Target/X86/X86CallLowering.cpp
+++ b/llvm/lib/Target/X86/X86CallLowering.cpp
@@ -105,7 +105,8 @@ struct X86OutgoingValueHandler : public CallLowering::OutgoingValueHandler {
STI(MIRBuilder.getMF().getSubtarget<X86Subtarget>()) {}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
LLT p0 = LLT::pointer(0, DL.getPointerSizeInBits(0));
LLT SType = LLT::scalar(DL.getPointerSizeInBits(0));
auto SPReg =
@@ -235,9 +236,15 @@ struct X86IncomingValueHandler : public CallLowering::IncomingValueHandler {
DL(MIRBuilder.getMF().getDataLayout()) {}
Register getStackAddress(uint64_t Size, int64_t Offset,
- MachinePointerInfo &MPO) override {
+ MachinePointerInfo &MPO,
+ ISD::ArgFlagsTy Flags) override {
auto &MFI = MIRBuilder.getMF().getFrameInfo();
- int FI = MFI.CreateFixedObject(Size, Offset, true);
+
+ // Byval is assumed to be writable memory, but other stack passed arguments
+ // are not.
+ const bool IsImmutable = !Flags.isByVal();
+
+ int FI = MFI.CreateFixedObject(Size, Offset, IsImmutable);
MPO = MachinePointerInfo::getFixedStack(MIRBuilder.getMF(), FI);
return MIRBuilder
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-stack-objects.ll b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-stack-objects.ll
new file mode 100644
index 000000000000..00eaeb9ae7a3
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-stack-objects.ll
@@ -0,0 +1,27 @@
+; RUN: llc -global-isel -mtriple=aarch64-unknown-unknown -stop-after=irtranslator -verify-machineinstrs %s -o - | FileCheck %s
+
+; The byval object should not be immutable, but the non-byval stack
+; passed argument should be.
+
+; CHECK-LABEL: name: stack_passed_i64
+; CHECK: fixedStack:
+; CHECK: - { id: 0, type: default, offset: 8, size: 8, alignment: 8, stack-id: default,
+; CHECK-NEXT: isImmutable: false, isAliased: false,
+; CHECK: - { id: 1, type: default, offset: 0, size: 8, alignment: 16, stack-id: default,
+; CHECK-NEXT: isImmutable: true, isAliased: false,
+define void @stack_passed_i64(i64 %arg, i64 %arg1, i64 %arg2, i64 %arg3, i64 %arg4, i64 %arg5, i64 %arg6,
+ i64 %arg7, i64 %arg8, i64* byval(i64) %arg9) {
+ ; CHECK: bb.1 (%ir-block.0):
+ ; CHECK: [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %fixed-stack.1
+ ; CHECK: [[LOAD:%[0-9]+]]:_(s64) = G_LOAD [[FRAME_INDEX]](p0) :: (invariant load 8 from %fixed-stack.1, align 16)
+ ; CHECK: [[FRAME_INDEX1:%[0-9]+]]:_(p0) = G_FRAME_INDEX %fixed-stack.0
+ ; CHECK: [[COPY8:%[0-9]+]]:_(p0) = COPY [[FRAME_INDEX1]](p0)
+ ; CHECK: [[LOAD1:%[0-9]+]]:_(s64) = G_LOAD [[COPY8]](p0) :: (dereferenceable load 8 from %ir.arg9)
+ ; CHECK: [[ADD:%[0-9]+]]:_(s64) = G_ADD [[LOAD1]], [[LOAD]]
+ ; CHECK: G_STORE [[ADD]](s64), [[COPY8]](p0) :: (volatile store 8 into %ir.arg9)
+ ; CHECK: RET_ReallyLR
+ %load = load i64, i64* %arg9
+ %add = add i64 %load, %arg8
+ store volatile i64 %add, i64* %arg9
+ ret void
+}
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-function-args.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-function-args.ll
index 22223e9a5eab..19ac9d9092a7 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-function-args.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-function-args.ll
@@ -1811,10 +1811,10 @@ define void @void_func_byval_i8_align32_i16_align64(i8 addrspace(5)* byval(i8) %
; CHECK: frameInfo:
; CHECK: maxAlignment: 64
; CHECK: fixedStack:
- ; CHECK: - { id: 0, type: default, offset: 64, size: 4, alignment: 16, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
- ; CHECK: - { id: 1, type: default, offset: 0, size: 4, alignment: 16, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+ ; CHECK: - { id: 0, type: default, offset: 64, size: 2, alignment: 16, stack-id: default,
+ ; CHECK-NEXT: isImmutable: false, isAliased: false, callee-saved-register: '',
+ ; CHECK: - { id: 1, type: default, offset: 0, size: 1, alignment: 16, stack-id: default,
+ ; CHECK-NEXT: isImmutable: false, isAliased: false, callee-saved-register: '',
; CHECK: bb.1 (%ir-block.0):
; CHECK: liveins: $sgpr30_sgpr31
; CHECK: [[FRAME_INDEX:%[0-9]+]]:_(p5) = G_FRAME_INDEX %fixed-stack.1
@@ -1843,10 +1843,10 @@ define void @byval_a3i32_align128_byval_i16_align64([3 x i32] addrspace(5)* byva
; CHECK: frameInfo:
; CHECK: maxAlignment: 128
; CHECK: fixedStack:
- ; CHECK-NEXT: - { id: 0, type: default, offset: 64, size: 4, alignment: 16, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
- ; CHECK: - { id: 1, type: default, offset: 0, size: 4, alignment: 16, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+ ; CHECK-NEXT: - { id: 0, type: default, offset: 64, size: 2, alignment: 16, stack-id: default,
+ ; CHECK-NEXT: isImmutable: false, isAliased: false, callee-saved-register: '',
+ ; CHECK: - { id: 1, type: default, offset: 0, size: 12, alignment: 16, stack-id: default,
+ ; CHECK-NEXT: isImmutable: false, isAliased: false, callee-saved-register: '',
; CHECK: bb.1 (%ir-block.0):
; CHECK: liveins: $sgpr30_sgpr31
; CHECK: [[FRAME_INDEX:%[0-9]+]]:_(p5) = G_FRAME_INDEX %fixed-stack.1
@@ -1887,10 +1887,10 @@ define void @void_func_v32i32_i32_byval_i8(<32 x i32> %arg0, i32 %arg1, i8 addrs
; CHECK: frameInfo:
; CHECK: maxAlignment: 8
; CHECK: fixedStack:
- ; CHECK: - { id: 0, type: default, offset: 8, size: 4, alignment: 8, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+ ; CHECK: - { id: 0, type: default, offset: 8, size: 1, alignment: 8, stack-id: default,
+ ; CHECK-NEXT: isImmutable: false, isAliased: false, callee-saved-register: '',
; CHECK: - { id: 1, type: default, offset: 0, size: 4, alignment: 16, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+ ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '',
; CHECK: bb.1 (%ir-block.0):
; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3, $vgpr4, $vgpr5, $vgpr6, $vgpr7, $vgpr8, $vgpr9, $vgpr10, $vgpr11, $vgpr12, $vgpr13, $vgpr14, $vgpr15, $vgpr16, $vgpr17, $vgpr18, $vgpr19, $vgpr20, $vgpr21, $vgpr22, $vgpr23, $vgpr24, $vgpr25, $vgpr26, $vgpr27, $vgpr28, $vgpr29, $vgpr30, $vgpr31, $sgpr30_sgpr31
; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
@@ -1951,9 +1951,9 @@ define void @void_func_v32i32_byval_i8_i32(<32 x i32> %arg0, i8 addrspace(5)* by
; CHECK: maxAlignment: 4
; CHECK: fixedStack:
; CHECK-NEXT: - { id: 0, type: default, offset: 4, size: 4, alignment: 4, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
- ; CHECK: - { id: 1, type: default, offset: 0, size: 4, alignment: 16, stack-id: default,
- ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+ ; CHECK-NEXT: isImmutable: true, isAliased: false, callee-saved-register: '',
+ ; CHECK: - { id: 1, type: default, offset: 0, size: 1, alignment: 16, stack-id: default,
+ ; CHECK-NEXT: isImmutable: false, isAliased: false, callee-saved-register: '',
; CHECK: bb.1 (%ir-block.0):
; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3, $vgpr4, $vgpr5, $vgpr6, $vgpr7, $vgpr8, $vgpr9, $vgpr10, $vgpr11, $vgpr12, $vgpr13, $vgpr14, $vgpr15, $vgpr16, $vgpr17, $vgpr18, $vgpr19, $vgpr20, $vgpr21, $vgpr22, $vgpr23, $vgpr24, $vgpr25, $vgpr26, $vgpr27, $vgpr28, $vgpr29, $vgpr30, $vgpr31, $sgpr30_sgpr31
; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
More information about the llvm-commits
mailing list