This is an archive of the discontinued LLVM Phabricator instance.

[InferAttrs] Add writeonly to all the math functions
ClosedPublic

Authored by bkramer on Dec 30 2021, 3:07 PM.

Details

Summary

All of these functions would be readnone, but can't be on platforms
where they can set errno. A writeonly function with no pointer
arguments can only write (but never read) global state.

Writeonly theoretically allows these calls to be CSE'd (a writeonly call
with the same arguments will always result in the same global stores) or
hoisted out of loops, but that's not implemented currently.

There are a few functions in this list that could be readnone instead
of writeonly, if someone is interested.

Diff Detail

Event Timeline

bkramer created this revision.Dec 30 2021, 3:07 PM
bkramer requested review of this revision.Dec 30 2021, 3:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 30 2021, 3:07 PM
reames resigned from this revision.Jan 2 2022, 9:08 AM

This looks reasonable, but I don't have the context on the math routines to know if this is 100% correct. Will defer to other reviewers.

fhahn accepted this revision.Jan 4 2022, 1:17 AM
fhahn added a subscriber: fhahn.

LGTM, thanks! This should be fine for math functions.

This revision is now accepted and ready to land.Jan 4 2022, 1:17 AM

+ same for math intrinsics? (possible followup?)

This revision was landed with ongoing or failed builds.Jan 4 2022, 7:58 AM
This revision was automatically updated to reflect the committed changes.

+ same for math intrinsics? (possible followup?)

The intrinsics are always readnone and pretend errno doesn't exist.

I went ahead and reverted this, as it broke compilation of a couple projects for me. The breakage is reproducible with this snippet:

$ cat sqrt.c
float a;  
b() { sqrt(a); }
$ clang -target x86_64-linux-gnu -c -O2 sqrt.c 
Attributes 'readnone and writeonly' are incompatible!
  %sqrtf = tail call float @sqrtf(float %0) #1  
in function b
fatal error: error in backend: Broken function found, compilation aborted!
uabelho added a subscriber: uabelho.Jan 5 2022, 2:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2022, 9:07 AM

What's up? This change landed in 5f0a349738304caf5f8083166f785a91048f574d.

Ah, okay, because there was no (reland) info here and today somehow I was pinged by mail to this patch. "Herald added a project: All.". Sorry.