Page MenuHomePhabricator

[PowerPC] in fixupIsDeadOrKill before RA, get def instructions by calling getVRegDef
ClosedPublic

Authored by shchenz on Aug 10 2020, 9:06 AM.

Details

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=47041

%1:gprc_and_gprc_nor0 = LI 0        ------>DefMI
%2:gprc = COPY %1:gprc_and_gprc_nor0
%3:gprc = ORI killed %2:gprc, 1      ------>MI

%3:gprc can be simplified to LI 1, so killed flag for %2:gprc should be hoisted up to other use or to def.
Because we set StartMI as DefMI which is through COPY like instructions before RA, so StartMI is %1:gprc_and_gprc_nor0 instead of %2:gprc, then when we fixup killed/dead flag for %2:gprc, we get %2:gprc redefine asserts.

This patch fixes this assertion, before RA, get real definition by calling getVRegDef instead of using StartMI.

Diff Detail

Event Timeline

shchenz created this revision.Aug 10 2020, 9:06 AM
shchenz requested review of this revision.Aug 10 2020, 9:06 AM
shchenz retitled this revision from [PowerPC] before RA, get def instructions by calling getVRegDef to [PowerPC] in fixupIsDeadOrKill before RA, get def instructions by calling getVRegDef .
dim added a subscriber: dim.Aug 10 2020, 1:25 PM

For me, this fixes the assertion for both the original test case (a fully preprocessed ASTReader.cpp) and the minimized test case from https://bugs.llvm.org/show_bug.cgi?id=47041#c1

shchenz updated this revision to Diff 284535.Aug 10 2020, 5:17 PM

make sure StartMI is a def through COPY

gentle ping...

lkail accepted this revision.Aug 16 2020, 7:49 PM
lkail added a subscriber: lkail.

LGTM with nits.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
2676

nit: findRegisterUseOperandIdx(RegNo) < 0 -> readsRegister(RegNo)

llvm/lib/Target/PowerPC/PPCInstrInfo.h
579

Could you add more comment about the fact StartMI might be altered?

This revision is now accepted and ready to land.Aug 16 2020, 7:49 PM
lkail added inline comments.Aug 16 2020, 7:53 PM
llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
2677

Oops, should be findRegisterUseOperandIdx(RegNo) < 0 -> !readsRegister(RegNo)

This revision was landed with ongoing or failed builds.Aug 16 2020, 11:13 PM
This revision was automatically updated to reflect the committed changes.

Thanks for your review @lkail The nits are updated in the commit

hans added a subscriber: hans.Aug 18 2020, 2:52 AM

Pushed to 11.x as 8cf2c031632f88a44eea68b48235496cf1c5d6ec. Please let me know if there are any follow-ups.