Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Nov 2, 2025

LowerCallTo handles all of the ABI details, including the load of
implicit sret return to the expected result positions.

LowerCallTo handles all of the ABI details, including the load of
implicit sret return to the expected result positions.
@arsenm arsenm added the backend:ARM label Nov 2, 2025 — with Graphite App
Copy link
Contributor Author

arsenm commented Nov 2, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Nov 2, 2025

@llvm/pr-subscribers-backend-arm

Author: Matt Arsenault (arsenm)

Changes

LowerCallTo handles all of the ABI details, including the load of
implicit sret return to the expected result positions.


Full diff: https://github.com/llvm/llvm-project/pull/166040.diff

1 Files Affected:

  • (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+2-37)
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 6b0653457cbaf..3a00267395504 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -9869,32 +9869,12 @@ SDValue ARMTargetLowering::LowerFSINCOS(SDValue Op, SelectionDAG &DAG) const {
   assert(Subtarget->isTargetDarwin());
 
   Type *ArgTy = ArgVT.getTypeForEVT(*DAG.getContext());
-  auto PtrVT = getPointerTy(DAG.getDataLayout());
-
-  MachineFrameInfo &MFI = DAG.getMachineFunction().getFrameInfo();
 
   // Pair of floats / doubles used to pass the result.
   Type *RetTy = StructType::get(ArgTy, ArgTy);
   auto &DL = DAG.getDataLayout();
 
   ArgListTy Args;
-  bool ShouldUseSRet = getTM().isAPCS_ABI();
-  SDValue SRet;
-  if (ShouldUseSRet) {
-    // Create stack object for sret.
-    const uint64_t ByteSize = DL.getTypeAllocSize(RetTy);
-    const Align StackAlign = DL.getPrefTypeAlign(RetTy);
-    int FrameIdx = MFI.CreateStackObject(ByteSize, StackAlign, false);
-    SRet = DAG.getFrameIndex(FrameIdx, getPointerTy(DL));
-
-    ArgListEntry Entry(SRet, PointerType::getUnqual(RetTy->getContext()));
-    Entry.IsSExt = false;
-    Entry.IsZExt = false;
-    Entry.IsSRet = true;
-    Args.push_back(Entry);
-    RetTy = Type::getVoidTy(*DAG.getContext());
-  }
-
   Args.emplace_back(Arg, ArgTy);
 
   StringRef LibcallName = getLibcallImplName(SincosStret);
@@ -9904,25 +9884,10 @@ SDValue ARMTargetLowering::LowerFSINCOS(SDValue Op, SelectionDAG &DAG) const {
   TargetLowering::CallLoweringInfo CLI(DAG);
   CLI.setDebugLoc(dl)
       .setChain(DAG.getEntryNode())
-      .setCallee(CC, RetTy, Callee, std::move(Args))
-      .setDiscardResult(ShouldUseSRet);
+      .setCallee(CC, RetTy, Callee, std::move(Args));
   std::pair<SDValue, SDValue> CallResult = LowerCallTo(CLI);
 
-  if (!ShouldUseSRet)
-    return CallResult.first;
-
-  SDValue LoadSin =
-      DAG.getLoad(ArgVT, dl, CallResult.second, SRet, MachinePointerInfo());
-
-  // Address of cos field.
-  SDValue Add = DAG.getNode(ISD::ADD, dl, PtrVT, SRet,
-                            DAG.getIntPtrConstant(ArgVT.getStoreSize(), dl));
-  SDValue LoadCos =
-      DAG.getLoad(ArgVT, dl, LoadSin.getValue(1), Add, MachinePointerInfo());
-
-  SDVTList Tys = DAG.getVTList(ArgVT, ArgVT);
-  return DAG.getNode(ISD::MERGE_VALUES, dl, Tys,
-                     LoadSin.getValue(0), LoadCos.getValue(0));
+  return CallResult.first;
 }
 
 SDValue ARMTargetLowering::LowerWindowsDIVLibCall(SDValue Op, SelectionDAG &DAG,

@arsenm arsenm requested review from MacDue and davemgreen November 2, 2025 06:05
@arsenm arsenm marked this pull request as ready for review November 2, 2025 06:05
Copy link
Member

@MacDue MacDue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though I think it'd be worth regenerating the llvm/test/CodeGen/ARM/sincos.ll test case (it looks pretty weak as it only matches bl ___sincosf_stret).

@arsenm arsenm merged commit a522ae3 into main Nov 3, 2025
14 checks passed
@arsenm arsenm deleted the users/arsenm/arm/remove-unnecessary-sincos-abi-lowering-code branch November 3, 2025 22:17
arsenm added a commit that referenced this pull request Nov 3, 2025
…166040)"

This reverts commit a522ae3.

The ABI handling doesn't account for matching the C ABI, only implicit
sret.
arsenm added a commit that referenced this pull request Nov 4, 2025
…166040)" (#166262)

This reverts commit a522ae3.

The ABI handling doesn't account for matching the C ABI, only implicit
sret.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants