This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Don't do the folding if the operand is R0/X0
ClosedPublic

Authored by steven.zhang on Mar 29 2020, 10:01 PM.

Details

Summary

We have this transformation in PowerPC peephole:

Replace instruction:
  renamable $x28 = ADDI8 renamable $x7, -8
  renamable $x28 = ADD8 killed renamable $x28, renamable $x0
  STFD killed renamable $f0, -8, killed renamable $x28 :: (store 8 into %ir._ind_cast99.epil)
with:
  renamable $x28 = ADDI8 renamable $x7, -16
  STFDX killed renamable $f0, $x0, killed $x28 :: (store 8 into %ir._ind_cast99.epil)

It is invalid as the '$x0' in STFDX is constant 0, not register r0.

stfdx FRS,RA,RB
if RA = 0 then b <- 0
else b <- (RA)
EA <- b +tea (RB)
MEM(EA, 8) <- (FRS)
MEMtag(EA, 8) <- 0

Diff Detail

Event Timeline

steven.zhang created this revision.Mar 29 2020, 10:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2020, 10:01 PM
nemanjai accepted this revision.Mar 30 2020, 3:27 AM

Ugh...
So we have a transformation that is called foldFrameOffset() and it would actually transform the attached test case? There does not appear to be any FrameIndex in the attached test case. So in addition to a missed check for R0/X0 vs. PPC::ZERO/PPC::ZERO8 we also have a transformation that says it folds frame offsets, but it does not really care about frame offsets - it just folds arithmetic into the index register.

I am not going to suggest that this be corrected in this patch, but this is something we should be much more diligent with.

The patch LGTM. Thanks for fixing the bug.

This revision is now accepted and ready to land.Mar 30 2020, 3:27 AM
This revision was automatically updated to reflect the committed changes.