[ARM] Mark function calls as possibly changing FPSCR

[Varnike]

This patch does the same changes as D143001 for AArch64.

This PR is part of the work on adding strict FP support in ARM, which was previously discussed in #137101.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-backend-arm

Author: Erik Enikeev (Varnike)

Changes

This patch does the same changes as D143001 for AArch64.

This PR is part of the work on adding strict FP support in ARM, which was previously discussed in #137101.

---

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

3 Files Affected:

• (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+5)
• (modified) llvm/lib/Target/ARM/ARMISelLowering.h (+2)
• (added) llvm/test/CodeGen/ARM/strict-fp-func.ll (+13)

diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 9052cbfa89deb..4f9616f09b4ea 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -22003,6 +22003,11 @@ bool ARMTargetLowering::isComplexDeinterleavingOperationSupported(
           ScalarTy->isIntegerTy(32));
 }
 
+ArrayRef<MCPhysReg> ARMTargetLowering::getRoundingControlRegisters() const {
+  static const MCPhysReg RCRegs[] = {ARM::FPSCR};
+  return RCRegs;
+}
+
 Value *ARMTargetLowering::createComplexDeinterleavingIR(
     IRBuilderBase &B, ComplexDeinterleavingOperation OperationType,
     ComplexDeinterleavingRotation Rotation, Value *InputA, Value *InputB,
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.h b/llvm/lib/Target/ARM/ARMISelLowering.h
index 8e417ac3e1a7b..7fd4c0b1fb2f5 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.h
+++ b/llvm/lib/Target/ARM/ARMISelLowering.h
@@ -995,6 +995,8 @@ class VectorType;
 
     bool isUnsupportedFloatingType(EVT VT) const;
 
+    ArrayRef<MCPhysReg> getRoundingControlRegisters() const override;
+
     SDValue getCMOV(const SDLoc &dl, EVT VT, SDValue FalseVal, SDValue TrueVal,
                     SDValue ARMcc, SDValue Flags, SelectionDAG &DAG) const;
     SDValue getARMCmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
diff --git a/llvm/test/CodeGen/ARM/strict-fp-func.ll b/llvm/test/CodeGen/ARM/strict-fp-func.ll
new file mode 100644
index 0000000000000..198d4fdea6831
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/strict-fp-func.ll
@@ -0,0 +1,13 @@
+; RUN: llc -mtriple aarch64-none-linux-gnu -stop-after=finalize-isel %s -o - | FileCheck %s
+
+define float @func_02(float %x, float %y) strictfp nounwind {
+  %call = call float @func_01(float %x) strictfp
+  %res = call float @llvm.experimental.constrained.fadd.f32(float %call, float %y, metadata !"round.dynamic", metadata !"fpexcept.ignore") strictfp
+  ret float %res
+}
+; CHECK-LABEL: name: func_02
+; CHECK:       BL @func_01, {{.*}}, implicit-def $fpcr
+
+
+declare float @func_01(float)
+declare float @llvm.experimental.constrained.fadd.f32(float, float, metadata, metadata)

[davemgreen]

I think this LGTM. Thanks

Are you happy for me to merge?

[statham-arm]

This test is using an AArch64 triple, even though the rest of the patch is modifying the ARM backend. Surely a mistake? It's testing the version of this functionality in a different backend. Also visible in the fact that the expected output refers to $fpcr, even though you defined the rounding control register as FPSCR above.

[Varnike]

Yes, this test should definitely use arm triple. It was taken from an AArch64 test set, and I forgot to update its contents. Thanks for noticing.

[statham-arm]

I don't quite understand why this is specified as the full FPSCR, when your other PR #160698 introduced FPSCR_RM which is specifically the rounding mode control bits?

[statham-arm]

Er, I should actually put the "request changes" label on this, due to the AArch64 test. Just in case it gets accidentally merged.

[Varnike]

As @statham-arm suggested, it would be more correct to use FPSCR_RM instead of FPSCR. However this leads to the fact that this patch becomes dependent on #160698, so it won't be possible to commit it for now.

[Varnike]

Rebased on main. Since #160698 has been merged upstream, this patch is now ready to be merged.