This is an archive of the discontinued LLVM Phabricator instance.

Attributor: Fix not propagating nofpclass arguments through transitive callers
ClosedPublic

Authored by arsenm on Aug 28 2023, 9:12 AM.

Details

Summary

Fixes #64867

Diff Detail

Event Timeline

arsenm created this revision.Aug 28 2023, 9:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 28 2023, 9:12 AM
Herald added subscribers: hoy, ormris, okura and 2 others. · View Herald Transcript
arsenm requested review of this revision.Aug 28 2023, 9:12 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: wdng. · View Herald Transcript
jdoerfert added inline comments.Aug 28 2023, 10:15 AM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
10350–10365

Do we really follow any use?

arsenm added inline comments.Aug 29 2023, 4:41 PM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
10350–10365

The only uses I could think of being worth skipping would be stores or void calls, which would naturally have no uses.

Maybe unrecognizable generic calls? Filtering out those is more effort if you want to consider libcalls

jdoerfert added inline comments.Aug 29 2023, 10:36 PM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
10350–10365

I mean, this could just flow into a call, what has the call return to do with the value we are tracking, right?
This could also flow into a fp2int and then into all sorts of things.

arsenm updated this revision to Diff 554872.Aug 30 2023, 5:32 PM

try to filter some values

jdoerfert added inline comments.Sep 1 2023, 11:15 AM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
10350–10365

So, we want it to follow fadd and friends, is that correct?
Does the qualification of the operation result apply to the operands all the time?

arsenm added inline comments.Sep 2 2023, 5:50 AM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
10350–10365

Should be anything that computeKnownFPClass is implemented for, which ideally would be all recognized FP operations

arsenm added inline comments.Sep 2 2023, 5:54 AM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
10350–10365

Probably could also filter out cases where we don't know anything about the value

arsenm updated this revision to Diff 555593.Sep 2 2023, 7:55 AM

Rebase tests. Apparently trying to skip uses if nothing is known doesn't work well

This revision is now accepted and ready to land.Sep 26 2023, 2:54 PM