Page MenuHomePhabricator

[PowerPC] Remove implicit use register after transformToImmFormFedByLI()
Needs ReviewPublic

Authored by lkail on Aug 5 2020, 3:11 AM.

Details

Reviewers
nemanjai
jsji
efriedma
ZhangKang
Group Reviewers
Restricted Project
Summary

When the instruction has imm form and fed by LI, we can remove the redundat LI instruction.
Below is an example:

renamable $x5 = LI8 2
renamable $x4 = exact SRD killed renamable $x4, killed renamable $r5, implicit $x5

will be converted to:

renamable $x5 = LI8 2
renamable $x4 = exact RLDICL killed renamable $x4, 62, 2,  implicit killed $x5

But when we do this optimization, we forget to remove implicit killed $x5
This bug has caused a lnt case error. This patch is to fix above bug.

Diff Detail

Event Timeline

ZhangKang created this revision.Aug 5 2020, 3:11 AM
ZhangKang requested review of this revision.Aug 5 2020, 3:11 AM
ZhangKang edited the summary of this revision. (Show Details)
ZhangKang edited the summary of this revision. (Show Details)
lkail commandeered this revision.Sep 9 2020, 11:34 PM
lkail added a reviewer: ZhangKang.
lkail planned changes to this revision.EditedMar 23 2021, 9:49 PM

Current fix looks not in a proper direction. If I remove lines llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp:[2259-2261) in https://reviews.llvm.org/D56078, the test in this patch gets passed.
I tried dump MI and its explicit_operands, the output looks not what I expected.
Code modification

if (SpecialShift32 || SpecialShift64) {
  MI.dump();
  for (auto &op : MI.explicit_operands()) {
    op.dump();
  }

Output

  renamable $x4 = exact RLDICL killed renamable $x4, killed renamable $r5, implicit $x5
renamable $x4
killed renamable $x4
killed renamable $r5
implicit $x5

implicit $x5 is in the explicit operand list.

lkail updated this revision to Diff 333722.Mar 28 2021, 7:27 AM

Do not early return in PPCInstrInfo::replaceInstrOperandWithImm.

implicit $x5 is in the explicit operand list.

Should it be an issue that the implicit $x5 should not be in the explicit operand list? Since when we have implicit $x5 as an explicit operand?

lkail added a comment.EditedApr 13 2021, 10:45 PM

implicit $x5 is in the explicit operand list.

Should it be an issue that the implicit $x5 should not be in the explicit operand list? Since when we have implicit $x5 as an explicit operand?

Take

renamable $x5 = LI8 2
renamable $x4 = exact SRD killed renamable $x4, killed renamable $r5, implicit $x5

converted to

renamable $x5 = LI8 2
renamable $x4 = exact RLDICL killed renamable $x4, 62, 2,  implicit killed $x5

as an example.
SRD has 2 explicit operands. When SRD is converted to RLDICL via setting a new MCID and RLDICL has 3 explicit operands, so implicit $x5 is within RLDICL's explicit operands. Later replaceInstrOperandWithImm is called and killed renamable $r5 is converted to imm 62. With origin impl, this instruction currently is

renamable $x4 = exact RLDICL killed renamable $x4, 62, implicit killed $x5

Then MachineInstrBuilder(*MI.getParent()->getParent(), MI).addImm(ME); is called. Notice that addImm's implementation will traverse the operand list and find a location explicit operand goes before implicit operand, so for now this instruction is

renamable $x4 = exact RLDICL killed renamable $x4, 62, 2,  implicit killed $x5

Thanks for the detailed explanation. I am thinking the replaceInstrOperandWithImm is functionality complete.
The issue here is checking an empty implicit operand set by MI.implicit_operands().empty() is not right as the MI is in the construction phase after calling setDesc()

Can we add a general function in MachineInstr class to check the empty implicit operand set by iterating all the operands?