This is an archive of the discontinued LLVM Phabricator instance.

[flang] Avoid unnecessary temporaries in ArrayValueCopy.
ClosedPublic

Authored by vzakhari on Jan 20 2023, 9:57 PM.

Details

Summary

Assume no conflict between pointer arrays and arrays without the target
attribute, if the fact of an array not having the target attribute
can be reliably computed.

This change speeds up SPEC CPU2017/527.cam from 2.5k seconds to 880 seconds
on Icelake, and makes further performance investigation easier.

Diff Detail

Event Timeline

vzakhari created this revision.Jan 20 2023, 9:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2023, 9:57 PM
vzakhari requested review of this revision.Jan 20 2023, 9:57 PM
vzakhari updated this revision to Diff 491034.Jan 20 2023, 10:02 PM
tschuett added inline comments.
flang/lib/Optimizer/Dialect/FIROps.cpp
3634

*mayHaveAttr instead of mayHaveAttr.value()

jeanPerier accepted this revision.Jan 23 2023, 12:50 AM
jeanPerier added inline comments.
flang/lib/Optimizer/Dialect/FIROps.cpp
3634

Better, you can combine the two lines above with return mayHaveAttr.value_or(true);

flang/test/Fir/array-value-copy-cam4.fir
39

Can you produce this test without the derived type descriptor data? I do not think they are relevant here and they make the test really hard to read. (you can generate the FIR with bbc -emit-fir -I nowhere where "nowhere" can literally be nowhere (it just have to be a directory that does not contain the intrinsic modules so that the front-end cannot generate the derived type descriptors) )

This revision is now accepted and ready to land.Jan 23 2023, 12:50 AM
PeteSteinfeld accepted this revision.Jan 23 2023, 7:29 AM

Other than a few nits on comments, all builds and tests correctly and looks good.

flang/lib/Optimizer/Dialect/FIROps.cpp
3546–3547

I couldn't mentally parse the sentence beginning with "Function argument ...". Does this make sense -- "Function arguments may also contribute to the value's definition and carry relevant attributes."

vzakhari marked 2 inline comments as done.Jan 23 2023, 9:58 AM
vzakhari added inline comments.
flang/lib/Optimizer/Dialect/FIROps.cpp
3546–3547

Thank you, Pete. I am not sure if "contribute" is the right word here. Please see if the modified version reads better.

flang/test/Fir/array-value-copy-cam4.fir
39

Sure! Fixed.

vzakhari updated this revision to Diff 491428.Jan 23 2023, 9:58 AM
This revision was automatically updated to reflect the committed changes.