This is an archive of the discontinued LLVM Phabricator instance.

[mlir][LLVM] Support locations in loop annotation
ClosedPublic

Authored by Dinistro on May 4 2023, 8:37 AM.

Details

Summary

This commit introduces support for locations as part of the loop
annotation attribute. These locations indicate the start and the end of
the loop.

Diff Detail

Event Timeline

Dinistro created this revision.May 4 2023, 8:37 AM
Herald added a reviewer: dcaballe. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Dinistro requested review of this revision.May 4 2023, 8:37 AM
Dinistro updated this revision to Diff 519747.May 4 2023, 11:18 PM

fix tests

gysit accepted this revision.May 4 2023, 11:34 PM

LGTM modulo nit comments.

mlir/lib/Target/LLVMIR/LoopAnnotationImporter.cpp
26

nit: for converting member functions or for the conversion of member functions?

413

Is FailureOr here needed? It seems like we use nullptr to signal failure?

424

I assume LLVM models same start and end location by adding only one location? Should we just set start and end location to the same value in that case?

This revision is now accepted and ready to land.May 4 2023, 11:34 PM
Dinistro updated this revision to Diff 519755.May 5 2023, 12:15 AM
Dinistro marked 3 inline comments as done.

address reviewer comments

mlir/lib/Target/LLVMIR/LoopAnnotationImporter.cpp
413

For the start loc, this is indeed not required. I added it to be compatible with the templated attribute construction further down.

424

From the LangRef (https://llvm.org/docs/LangRef.html#llvm-loop):

Prior to the property nodes, one or two DILocation (debug location) nodes can be present in the list. The first, if present, identifies the source-code location where the loop begins. The second, if present, identifies the source-code location where the loop ends.

So the second one really indicates the end.
I added an error when there are more than two locations, though.

This revision was automatically updated to reflect the committed changes.