Page MenuHomePhabricator

[FlattenCFG] Fix `MergeIfRegion` in case then-path is empty
ClosedPublic

Authored by ekatz on Apr 26 2020, 9:50 AM.

Details

Summary

In case the then-path of an if-region is empty, then merging with the else-path should be handled with the inverse of the condition (leading to that path).

Fix PR37662

Diff Detail

Event Timeline

ekatz created this revision.Apr 26 2020, 9:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2020, 9:50 AM
kuhar accepted this revision.May 11 2020, 6:55 AM

I followed the new tests and the patch seems fine to me, but I'd rather someone else took a look too.

llvm/lib/Transforms/Utils/FlattenCFG.cpp
487

Is there a better name for Invert2? I think until this point it's difficult to tell what it's actually for.

llvm/test/Transforms/Util/flattencfg.ll
32

Good catch!

This revision is now accepted and ready to land.May 11 2020, 6:55 AM
ekatz marked an inline comment as done.May 12 2020, 12:08 AM
ekatz added inline comments.
llvm/lib/Transforms/Utils/FlattenCFG.cpp
487

Sure, I'll find a better name.

ekatz updated this revision to Diff 263391.May 12 2020, 3:15 AM
ekatz edited the summary of this revision. (Show Details)

Rename Invert2 variable to InvertCond2.
Hope it makes more sense.

kuhar accepted this revision.May 12 2020, 8:17 AM

Rename Invert2 variable to InvertCond2.
Hope it makes more sense.

Thanks!

sameerds accepted this revision.May 20 2020, 10:40 PM

I am NOT familiar with this transformation. But I believe that the patch is well-formed, and I was able to follow the logic. The tests also demonstrate that it works as intended. LGTM.

This revision was automatically updated to reflect the committed changes.