Page MenuHomePhabricator

[branchfolding] When hoisting common code, remove kill flags from uses
Needs ReviewPublic

Authored by Ka-Ka on Apr 25 2018, 3:02 AM.

Details

Summary

The branch folder wrongly leave kill flags that the verifier
complain about. This can only occur on targets with super
registers where you can write the parts individually. The reason
for this is that the function addRegAndItsAliases() that build
the ActiveDefsSet will consider the super register defed if one
of the part are defed. The easy way to solve this is to be
conservative and always remove kill flags from uses if the killed
registers are needed below the insertion point, regardless of if
registers exist in the ActiveDefsSet or not.

This is a conservative refinement of the old fix in r148043

Diff Detail

Event Timeline

Ka-Ka created this revision.Apr 25 2018, 3:02 AM
jonpa added a subscriber: jonpa.

The test case seems to work fine when I tried it w and w/out the patch.

Adding Uli as a reviewer for the test case, since it's SystemZ (he always reviews SystemZ changes). Note: as Kalle explained to me in a mail, this test case has been somewhat modified to expose the bug on SystemZ, but since this is a .mir test case which passes the machine verifier, I think this is not a problem.

Some reviewer for BranchFolding.cpp still needed.

The test case looks fine to me, thanks.

Ka-Ka added a comment.Apr 26 2018, 2:10 AM

Adding Uli as a reviewer for the test case, since it's SystemZ (he always reviews SystemZ changes). Note: as Kalle explained to me in a mail, this test case has been somewhat modified to expose the bug on SystemZ, but since this is a .mir test case which passes the machine verifier, I think this is not a problem.

The fault was originally found in a out of tree target, but I was able to reproduce the fault on SystemZ as my out of tree target and SystemZ share the same register characteristics (the register r0d is a pair of r0h and r0l and they can be written to separately) needed to trigger the fault.

Adding Matthias Braun and Krzysztof Parzyszek as reviewer as it seems you both have done work in BranchFolding.cpp before. Do you have time to review this change?

Ka-Ka edited the summary of this revision. (Show Details)May 7 2018, 5:47 AM
Ka-Ka added a comment.May 14 2018, 1:47 AM

Ping!
Anyone up for reviewing this?