[DAG] Make strictfp attribute only restricts for libm and make non-math optimizations possible

[diggerlin]

the patch

Add strictfp attribute to prevent unwanted optimizations of libm calls

add I.isStrictFP() into

  if (!I.isNoBuiltin() && !I.isStrictFP() && !F->hasLocalLinkage() &&
        F->hasName() && LibInfo->getLibFunc(*F, Func) &&
        LibInfo->hasOptimizedCodeGen(Func)) 

it prevents the backend from optimizing even non-math libcalls such as strlen and memcmp if a call has the strict floating-point attribute. For example, it prevent converting strlen and memcmp to milicode call __strlen and __memcmp.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-powerpc

Author: zhijian lin (diggerlin)

Changes

the patch

commit 53a5fbb45fa45cba48963a6b17defa4c4f072d9d
Author: Andrew Kaylor &lt;andrew.kaylor@<!-- -->intel.com&gt;
Date:   Mon Aug 14 21:15:13 2017 +0000

    Add strictfp attribute to prevent unwanted optimizations of libm calls

    Differential Revision: https://reviews.llvm.org/D34163

add I.isStrictFP() into

  if (!I.isNoBuiltin() && !I.isStrictFP() && !F->hasLocalLinkage() &&
        F->hasName() && LibInfo->getLibFunc(*F, Func) &&
        LibInfo->hasOptimizedCodeGen(Func)) 

it prevents the backend from optimizing even non-math libcalls such as strlen and memcmp if a call has the strict floating-point attribute.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+9-6)
• (modified) llvm/test/CodeGen/PowerPC/milicode32.ll (+2-1)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index a52265055c88a..37108aa3a56d8 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -9392,7 +9392,9 @@ bool SelectionDAGBuilder::visitStrNLenCall(const CallInst &I) {
 bool SelectionDAGBuilder::visitUnaryFloatCall(const CallInst &I,
                                               unsigned Opcode) {
   // We already checked this call's prototype; verify it doesn't modify errno.
-  if (!I.onlyReadsMemory())
+  // Do not perform optimizations for call sites that require strict
+  // floating-point semantics.
+  if (!I.onlyReadsMemory() || I.isStrictFP())
     return false;
 
   SDNodeFlags Flags;
@@ -9412,7 +9414,9 @@ bool SelectionDAGBuilder::visitUnaryFloatCall(const CallInst &I,
 bool SelectionDAGBuilder::visitBinaryFloatCall(const CallInst &I,
                                                unsigned Opcode) {
   // We already checked this call's prototype; verify it doesn't modify errno.
-  if (!I.onlyReadsMemory())
+  // Do not perform optimizations for call sites that require strict
+  // floating-point semantics.
+  if (!I.onlyReadsMemory() || I.isStrictFP())
     return false;
 
   SDNodeFlags Flags;
@@ -9445,11 +9449,10 @@ void SelectionDAGBuilder::visitCall(const CallInst &I) {
 
     // Check for well-known libc/libm calls.  If the function is internal, it
     // can't be a library call.  Don't do the check if marked as nobuiltin for
-    // some reason or the call site requires strict floating point semantics.
+    // some reason.
     LibFunc Func;
-    if (!I.isNoBuiltin() && !I.isStrictFP() && !F->hasLocalLinkage() &&
-        F->hasName() && LibInfo->getLibFunc(*F, Func) &&
-        LibInfo->hasOptimizedCodeGen(Func)) {
+    if (!I.isNoBuiltin() && !F->hasLocalLinkage() && F->hasName() &&
+        LibInfo->getLibFunc(*F, Func) && LibInfo->hasOptimizedCodeGen(Func)) {
       switch (Func) {
       default: break;
       case LibFunc_bcmp:
diff --git a/llvm/test/CodeGen/PowerPC/milicode32.ll b/llvm/test/CodeGen/PowerPC/milicode32.ll
index 78d036202fe4e..09239fb9d1b09 100644
--- a/llvm/test/CodeGen/PowerPC/milicode32.ll
+++ b/llvm/test/CodeGen/PowerPC/milicode32.ll
@@ -64,8 +64,9 @@ entry:
   %str.addr = alloca ptr, align 4
   store ptr %str, ptr %str.addr, align 4
   %0 = load ptr, ptr %str.addr, align 4
-  %call = call i32 @strlen(ptr noundef %0)
+  %call = call i32 @strlen(ptr noundef %0) #0
   ret i32 %call
 }
 
 declare i32 @strlen(ptr noundef) nounwind
+attributes #0 = { strictfp }

[diggerlin]

Friendly reminder — any feedback on this patch when you have a moment?

[lei137]

LGTM.
Thx

[arsenm]

Should add test, not modify existing one

[arsenm]

bump

[diggerlin]

I’ve added a new test scenario called strlen_test_fp_strict , which differs from strlen_test.
there is

%call = call i32 @strlen(ptr noundef %0) #0 
attributes #0 = { strictfp }

in the strlen_test_fp_strict.
I’m not entirely sure what “bump” refers to in this context. could you please clarify its meaning?