AMDGPU: RETURNADDR lowering

[aakanksha555]

Summary

Patch to do return address lowering.

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

2793–2795 ↗

(On Diff #191776)

verifyReturnAddressArgumentIsConstant should probably be removed in a separate patch. The verifier now rejects the IR, and the DAG should never be allowed to produce an invalid one

2799–2801 ↗

(On Diff #191776)

Unnecessary comment

2807–2810 ↗

(On Diff #191776)

This should not be an assert. According to the spec, this could just return 0 for another frame id

2815 ↗

(On Diff #191776)

Should use TRI::getReturnAddressReg rather than hardcoding it in another place. You can just hardcode the register class though

test/CodeGen/AMDGPU/returnaddress.ll

1 ↗

(On Diff #191776)

Should use HSA triple

9 ↗

(On Diff #191776)

Use named values

12 ↗

(On Diff #191776)

Needs test with non-0 frame

[arsenm]

Commit message should also say it's lowering returnaddress, not mention instrument-functions

[arsenm]

test/CodeGen/AMDGPU/returnaddress.ll

7 ↗

(On Diff #191776)

Needs a test with amdgpu_kernel and shaders. As is this will fail as they don’t have return addresses

[arsenm]

lib/Target/AMDGPU/SIISelLowering.cpp

4156 ↗

(On Diff #198542)

Formatting

4159–4161 ↗

(On Diff #198542)

This can just give 0 instead of erring

4168 ↗

(On Diff #198542)

This still won't handle kernels correctly

test/CodeGen/AMDGPU/returnaddress.ll

19 ↗

(On Diff #198542)

Needs tests with a kernel and shader, since this should crash as is since there is no return address

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

29 ↗

(On Diff #198542)

Not sure why you need a new include here

[aakanksha555]

Return 0 for kernels and non-zero arguments to llvm.returnaddress.

[arsenm]

Mostly LGTM, but 2 more test cases might be useful; one with no ret, and one with the use outside of the entry block

[aakanksha555]

Added a test outside entry block; doesn't make any difference in the output.
By a test with no ret, do you mean something that returns void?

[aakanksha555]

Minor changes in one of the tests.

[arsenm]

In D59666#1502063, @aakanksha555 wrote:

Added a test outside entry block; doesn't make any difference in the output.
By a test with no ret, do you mean something that returns void?

No, I mean ends in unreachable

test/CodeGen/AMDGPU/returnaddress.ll

47–49 ↗

(On Diff #199841)

This is still in the entry block

[aakanksha555]

Fixed two tests; one which does not return and another with use outside of the entry block.

[arsenm]

returnadr.diff

2 ↗

(On Diff #201329)

You somehow added the diff as a file

105 ↗

(On Diff #201329)

The call is still in the entry block. You should also make it a conditional branch to make sure nothing folds it

test/CodeGen/AMDGPU/returnaddress.ll

42–43 ↗

(On Diff #201329)

The call is still in the entry block. You should also make it a conditional branch to make sure nothing folds it

[aakanksha555]

Fix in one of the tests.

[arsenm]

LGTM

test/CodeGen/AMDGPU/returnaddress.ll

60–62 ↗

(On Diff #201335)

This can just be one block

llvm/trunk/lib/Target/AMDGPU/SIISelLowering.h

private:
   SDValue LowerFDIV32(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV64(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerINT_TO_FP(SDValue Op, SelectionDAG &DAG, bool Signed) const;
   SDValue LowerSTORE(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerTrig(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerATOMIC_CMP_SWAP(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerBRCOND(SDValue Op, SelectionDAG &DAG) const;
-
+  SDValue LowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;
   SDValue adjustLoadValueType(unsigned Opcode, MemSDNode *M,
                               SelectionDAG &DAG, ArrayRef<SDValue> Ops,
                               bool IsIntrinsic = false) const;
 
   // Call DAG.getMemIntrinsicNode for a load, but first widen a dwordx3 type to
   // dwordx4 if on SI.
   SDValue getMemIntrinsicNode(unsigned Opcode, const SDLoc &DL, SDVTList VTList,
                               ArrayRef<SDValue> Ops, EVT MemVT,

llvm/trunk/lib/Target/AMDGPU/SIISelLowering.cpp

SDValue SITargetLowering::splitBinaryVectorOp(SDValue Op,
 
   return DAG.getNode(ISD::CONCAT_VECTORS, SDLoc(Op), VT, OpLo, OpHi);
 }
 
 SDValue SITargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
   switch (Op.getOpcode()) {
   default: return AMDGPUTargetLowering::LowerOperation(Op, DAG);
   case ISD::BRCOND: return LowerBRCOND(Op, DAG);
+  case ISD::RETURNADDR: return LowerRETURNADDR(Op, DAG);
   case ISD::LOAD: {
     SDValue Result = LowerLOAD(Op, DAG);
     assert((!Result.getNode() ||
             Result.getNode()->getNumValues() == 2) &&
            "Load should return a value and a chain");
     return Result;
   }
 
SDValue SITargetLowering::LowerBRCOND(SDValue BRCOND,
   // Remove the old intrinsic from the chain
   DAG.ReplaceAllUsesOfValueWith(
     SDValue(Intr, Intr->getNumValues() - 1),
     Intr->getOperand(0));
 
   return Chain;
 }
 
+SDValue SITargetLowering::LowerRETURNADDR(SDValue Op,
+                                          SelectionDAG &DAG) const {
+  MVT VT = Op.getSimpleValueType();
+  SDLoc DL(Op);
+  // Checking the depth
+  if (cast<ConstantSDNode>(Op.getOperand(0))->getZExtValue() != 0)
+    return DAG.getConstant(0, DL, VT);
+
+  MachineFunction &MF = DAG.getMachineFunction();
+  const SIMachineFunctionInfo *Info = MF.getInfo<SIMachineFunctionInfo>();
+  // Check for kernel and shader functions
+  if (Info->isEntryFunction())
+    return DAG.getConstant(0, DL, VT);
+
+  MachineFrameInfo &MFI = MF.getFrameInfo();
+  // There is a call to @llvm.returnaddress in this function
+  MFI.setReturnAddressIsTaken(true);
+
+  const SIRegisterInfo *TRI = getSubtarget()->getRegisterInfo();
+  // Get the return address reg and mark it as an implicit live-in
+  unsigned Reg = MF.addLiveIn(TRI->getReturnAddressReg(MF), getRegClassFor(VT, Op.getNode()->isDivergent()));
+
+  return DAG.getCopyFromReg(DAG.getEntryNode(), DL, Reg, VT);
+}
+
 SDValue SITargetLowering::getFPExtOrFPTrunc(SelectionDAG &DAG,
                                             SDValue Op,
                                             const SDLoc &DL,
                                             EVT VT) const {
   return Op.getValueType().bitsLE(VT) ?
       DAG.getNode(ISD::FP_EXTEND, DL, VT, Op) :
       DAG.getNode(ISD::FTRUNC, DL, VT, Op);
 }

llvm/trunk/test/CodeGen/AMDGPU/returnaddress.ll

+; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck --check-prefix=GCN %s
+
+; Test with zero frame
+; GCN-LABEL: {{^}}func1
+; GCN: v_mov_b32_e32 v0, s30
+; GCN: v_mov_b32_e32 v1, s31
+; GCN: s_setpc_b64 s[30:31]
+define i8* @func1() nounwind {
+entry:
+  %0 = tail call i8* @llvm.returnaddress(i32 0)
+  ret i8* %0
+}
+
+; Test with non-zero frame
+; GCN-LABEL: {{^}}func2
+; GCN: v_mov_b32_e32 v0, 0
+; GCN: v_mov_b32_e32 v1, 0
+; GCN: s_setpc_b64 s[30:31]
+define i8* @func2() nounwind {
+entry:
+  %0 = tail call i8* @llvm.returnaddress(i32 1)
+  ret i8* %0
+}
+
+; Test with amdgpu_kernel
+; GCN-LABEL: {{^}}func3
+; GCN: v_mov_b32_e32 v0, 0
+; GCN: v_mov_b32_e32 v1, v0
+define amdgpu_kernel void @func3(i8** %out) nounwind {
+entry:
+  %tmp = tail call i8* @llvm.returnaddress(i32 0)
+  store i8* %tmp, i8** %out, align 4
+  ret void
+}
+
+; Test with use outside the entry-block
+; GCN-LABEL: {{^}}func4
+; GCN: v_mov_b32_e32 v0, 0
+; GCN: v_mov_b32_e32 v1, v0
+define amdgpu_kernel void @func4(i8** %out, i32 %val) nounwind {
+entry:
+  %cmp = icmp ne i32 %val, 0
+  br i1 %cmp, label %store, label %exit
+
+store:
+  %tmp = tail call i8* @llvm.returnaddress(i32 1)
+  store i8* %tmp, i8** %out, align 4
+  ret void
+
+exit:
+  ret void
+}
+
+; Test ending in unreachable
+; GCN-LABEL: {{^}}func5
+; GCN: v_mov_b32_e32 v0, 0
+define void @func5() nounwind {
+entry:
+  %tmp = tail call i8* @llvm.returnaddress(i32 2)
+  store volatile i32 0, i32 addrspace(3)* undef, align 4
+  unreachable
+}
+
+
+declare i8* @llvm.returnaddress(i32) nounwind readnone