Page MenuHomePhabricator

Branch relaxation - non invertible condition
ClosedPublic

Authored by delena on Dec 29 2017, 1:40 PM.

Details

Summary

The algorithm of the branch relaxation is not fully correct and generates a wrong code when condition can't be inverted.
I changed the "fixupConditionalBranch()" function - a new BB (trampoline) is created to keen the original branch condition.

Apparently, I can't attach a test case, I'm working out-of-tree.

Diff Detail

Event Timeline

delena created this revision.Dec 29 2017, 1:40 PM
arsenm added inline comments.Dec 29 2017, 1:50 PM
../lib/CodeGen/BranchRelaxation.cpp
401–434 ↗(On Diff #128359)

This part is exactly the same as the other path above. Can you move this around to avoid duplicating it?

422–427 ↗(On Diff #128359)

These can all be one DEBUG() block

423 ↗(On Diff #128359)

Prints trailing whitespace

424 ↗(On Diff #128359)

'\n'

delena updated this revision to Diff 128359.Dec 30 2017, 8:11 AM

Re-arranged the code to avoid duplication.
Fixed Debug prints.

arsenm accepted this revision.Jan 3 2018, 10:13 AM

LGTM

This revision is now accepted and ready to land.Jan 3 2018, 10:13 AM

I understand that if it is a out-of-tree target, the testcase is problematic,
but no testcase at all is worrying. Maybe unit-test can help?

I understand that if it is a out-of-tree target, the testcase is problematic,
but no testcase at all is worrying. Maybe unit-test can help?

It is a machine instruction pass. My target can't reverse conditional branches with FP comparison.
I just don't know how to create a unit-test without TII that decides whether the condition may be reversed. Can you give me an example of MIR test without target?

kparzysz added inline comments.
../lib/CodeGen/BranchRelaxation.cpp
413 ↗(On Diff #128359)

Typo: br -> b.

I understand that if it is a out-of-tree target, the testcase is problematic,
but no testcase at all is worrying. Maybe unit-test can help?

It is a machine instruction pass. My target can't reverse conditional branches with FP comparison.
I just don't know how to create a unit-test without TII that decides whether the condition may be reversed. Can you give me an example of MIR test without target?

I'm sorry, but i'm just an observer in this case, i don't know anything about any of this code.
So i can't help, sorry.

This revision was automatically updated to reflect the committed changes.