This is an archive of the discontinued LLVM Phabricator instance.

[flang] Added coarse grained alias analysis for FIR.
ClosedPublic

Authored by vzakhari on Jan 10 2023, 10:34 AM.

Details

Summary

These are experimental changes in Flang AA to provide
at least some means to disambiguate memory accesses in some
simple cases. This AA is still not used by any transformation,
so the LIT tests are the only way to trigger it currently.
I will further look into applying this AA within Flang
to address some of the known performance issues in the benchmarks.

Credits to @Renaud-K for the initial implementation.

Diff Detail

Event Timeline

vzakhari created this revision.Jan 10 2023, 10:34 AM
vzakhari requested review of this revision.Jan 10 2023, 10:34 AM
PeteSteinfeld accepted this revision.Jan 10 2023, 1:30 PM

Aside from a couple of nits, all builds and tests correctly and looks good.

flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
40

Should read "unknown" rather than "uknown".

65–68

This name would read a little more naturally if it were isRecordWithPointerComponent. But this function isn't mentioned anywhere else. Can you just get rid of it?

flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
128–137

This code looks identical to the code in lines 95 -104. Should they be extracted to a separate function?

This revision is now accepted and ready to land.Jan 10 2023, 1:30 PM
vzakhari added inline comments.Jan 10 2023, 1:44 PM
flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
40

Thanks!

65–68

Good name suggestion! Thanks!

flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
128–137

I would like to keep it as-is for the time being. This code is WIP, so I will likely restructure it in future. So far, the code duplication is not too awful :)

vzakhari updated this revision to Diff 487975.Jan 10 2023, 1:45 PM
  • Fixed typo and the method name.
tschuett added inline comments.
flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
27

static. No anonymous namespace for functions.

106

The common style for asserts in LLVM is assert(important && "something bad happened ");

vzakhari updated this revision to Diff 487982.Jan 10 2023, 1:57 PM
vzakhari marked 2 inline comments as done.
tschuett added inline comments.Jan 10 2023, 2:38 PM
flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
46

My last question: struct or class?

vzakhari added inline comments.Jan 10 2023, 3:54 PM
flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
46

struct should be good enough.

jeanPerier accepted this revision.Jan 11 2023, 12:32 AM

Looks great !

This revision was automatically updated to reflect the committed changes.
tblah added a subscriber: tblah.Jan 12 2023, 2:05 AM