This is an archive of the discontinued LLVM Phabricator instance.

[PatternMatch] Make m_Br more flexible, add matchers for BB values.
ClosedPublic

Authored by fhahn on Sep 25 2019, 3:44 AM.

Details

Summary

Currently m_Br only takes references to BasicBlock*, which limits its
flexibility. For example, you have to declare a variable, even if you
ignore the result or you have to have additional checks to make sure the
matched BB matches an expected one.

This patch adds m_BasicBlock and m_SpecificBB matchers, which can be
used like the existing matchers for constants or values.

I also had a look at the existing uses and updated a few. IMO it makes
the code a bit more explicit.

Event Timeline

fhahn created this revision.Sep 25 2019, 3:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2019, 3:44 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
lebedev.ri added inline comments.Sep 25 2019, 3:51 AM
llvm/include/llvm/IR/PatternMatch.h
682–686

Should m_DeferredBB() be added at the same time?

fhahn updated this revision to Diff 221718.Sep 25 2019, 4:22 AM

Add m_Deferred overload for BasicBlock & test.

fhahn marked an inline comment as done.Sep 25 2019, 4:24 AM
fhahn added inline comments.
llvm/include/llvm/IR/PatternMatch.h
682–686

Nice, I did not know about m_Deferred!

I've added an overloaded version for BasicBlock, not sure if there's a benefit for including BB in the name, as we also don't include Value in the Value variant name. I've also added a test.

lebedev.ri added inline comments.Sep 25 2019, 4:48 AM
llvm/include/llvm/IR/PatternMatch.h
682–683

This comment seems outdated.

1366–1371

Precommit

fhahn updated this revision to Diff 221741.Sep 25 2019, 6:22 AM

Fix comment and split off suggested changes for precommit (once the patch is accepted).

fhahn updated this revision to Diff 221743.Sep 25 2019, 6:26 AM
fhahn marked 3 inline comments as done.

Move another bit to precommit.

Harbormaster completed remote builds in B38534: Diff 221743.
fhahn marked an inline comment as done.Sep 25 2019, 6:26 AM
fhahn added inline comments.
llvm/include/llvm/IR/PatternMatch.h
682–683

Thanks, updated!

1366–1371

Done, split off to a separate commit, to be committed once this one is accepted.

lebedev.ri accepted this revision.Sep 25 2019, 6:58 AM

Looks good to me.

llvm/include/llvm/IR/PatternMatch.h
1395

Should there be m_c_Br ?

This revision is now accepted and ready to land.Sep 25 2019, 6:58 AM
fhahn marked an inline comment as done.Sep 25 2019, 7:52 AM

Thanks!

llvm/include/llvm/IR/PatternMatch.h
1395

I am not entirely sure. I think for most use cases with branches, matching commuted targets is not too common, but if there are cases it should be a straight forward follow-up.

This revision was automatically updated to reflect the committed changes.