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).

diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h b/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
index 0d35e42..0d587c1 100644 (file)
--- a/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
@@ -149,7 +149,8 @@ public:
     /// 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 946950d..f689801 100644 (file)
--- 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 ce1ec90..d16e2fd 100644 (file)
--- 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 c0571e4..e196778 100644 (file)
--- 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 d4b920a..fe8552c 100644 (file)
--- 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 d5251ce..9cd6f7f 100644 (file)
--- 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 28bba84..bebc654 100644 (file)
--- 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 (file)
index 0000000..00eaeb9
--- /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 22223e9..19ac9d9 100644 (file)
--- 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