Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+43 −6
Open

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a40b5e1
let strictfp only work for libm
diggerlin Oct 28, 2025
44c7bad
add comment to add test caee for fp-strict
diggerlin Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion llvm/test/CodeGen/PowerPC/milicode32.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@arsenm arsenm Nov 3, 2025

Choose a reason for hiding this comment

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

Should add test, not modify existing one

Copy link
Contributor

@arsenm arsenm Nov 4, 2025

Choose a reason for hiding this comment

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

bump

Copy link
Contributor Author

@diggerlin diggerlin Nov 5, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@diggerlin diggerlin Nov 6, 2025

Choose a reason for hiding this comment

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

Whenever you have a moment, could you please take another look and let me know your thoughts —especially regarding the “bump” reference I mentioned?
Thanks for your time! @arsenm

ret i32 %call
}

declare i32 @strlen(ptr noundef) nounwind
attributes #0 = { strictfp }