This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Remove the repeated definition for some InstAlias for mtspr/mfspr
ClosedPublic

Authored by ZhangKang on Mar 7 2020, 11:12 PM.

Details

Summary

Below InstAlias have been redefined, this patch is to remove the repeated definition.
mtdec/mfdec mtsdr1/mfsdr1 mtsrr0/mfsrr0 mtsrr1/mfsrr1 mtasr

4517 def : InstAlias<"mtdec $Rx", (MTSPR 22, gprc:$Rx)>;
4518 def : InstAlias<"mfdec $Rx", (MFSPR gprc:$Rx, 22)>;
4519
4520 def : InstAlias<"mtsdr1 $Rx", (MTSPR 25, gprc:$Rx)>;
4521 def : InstAlias<"mfsdr1 $Rx", (MFSPR gprc:$Rx, 25)>;
4522
4523 def : InstAlias<"mtsrr0 $Rx", (MTSPR 26, gprc:$Rx)>;
4524 def : InstAlias<"mfsrr0 $Rx", (MFSPR gprc:$Rx, 26)>;
4525
4526 def : InstAlias<"mtsrr1 $Rx", (MTSPR 27, gprc:$Rx)>;
4527 def : InstAlias<"mfsrr1 $Rx", (MFSPR gprc:$Rx, 27)>;
4528
4529 def : InstAlias<"mtcfar $Rx", (MTSPR 28, gprc:$Rx)>;
4530 def : InstAlias<"mfcfar $Rx", (MFSPR gprc:$Rx, 28)>;
...

4557 def : InstAlias<"mtasr $RT", (MTSPR 280, gprc:$RT)>;

...
4621 def : InstAlias<"mtasr $RS", (MTSPR 280, gprc:$RS)>;
4622
4623 def : InstAlias<"mfdec $RT", (MFSPR gprc:$RT, 22)>;
4624 def : InstAlias<"mtdec $RT", (MTSPR 22, gprc:$RT)>;
4625
4626 def : InstAlias<"mfsdr1 $RT", (MFSPR gprc:$RT, 25)>;
4627 def : InstAlias<"mtsdr1 $RT", (MTSPR 25, gprc:$RT)>;
4628
4629 def : InstAlias<"mfsrr0 $RT", (MFSPR gprc:$RT, 26)>;
4630 def : InstAlias<"mfsrr1 $RT", (MFSPR gprc:$RT, 27)>;
4631 def : InstAlias<"mtsrr0 $RT", (MTSPR 26, gprc:$RT)>;
4632 def : InstAlias<"mtsrr1 $RT", (MTSPR 27, gprc:$RT)>;

Diff Detail

Event Timeline

ZhangKang created this revision.Mar 7 2020, 11:12 PM
ZhangKang edited the summary of this revision. (Show Details)Mar 7 2020, 11:17 PM
ZhangKang updated this revision to Diff 248987.Mar 8 2020, 12:00 AM

Remove mtasr.

ZhangKang edited the summary of this revision. (Show Details)Mar 8 2020, 12:05 AM
ZhangKang set the repository for this revision to rG LLVM Github Monorepo.
ZhangKang updated this revision to Diff 248992.Mar 8 2020, 5:13 AM
ZhangKang edited the summary of this revision. (Show Details)

Rebase the latest code.

This is a curious situation. How did we end up having multiple definitions of instruction aliases?

This is a curious situation. How did we end up having multiple definitions of instruction aliases?

These were added six years ago in different time. The code is added from different patch. The one is https://reviews.llvm.org/rG62cb635 Implement asm support for a few PowerPC bookIII that are needed for assembling in 2013.9, and the other is https://reviews.llvm.org/rGb1ccf56 Add a number of aliases for SPR access in 2014.7.

Before my NFC patch https://reviews.llvm.org/rGb0f3d49a05ce487650ac0b3e3a23eae21f494a96 , the alias definition for MTSPR/MFSPR is unordered. The second author added the repeated code whether search whether there is the same definition.

steven.zhang added a comment.EditedMar 9 2020, 6:44 PM

This is a curious situation. How did we end up having multiple definitions of instruction aliases?

If there is multiple definitions of instruction aliases, there will be two identical entries in the match table except the aliased mnemonic generated by tblgen. Maybe, tblgen need to be improved to diagnose such case.

steven.zhang accepted this revision.Mar 19 2020, 7:22 PM

LGTM (but see if Nemanjai(@nemanjai ) has comments)

This revision is now accepted and ready to land.Mar 19 2020, 7:22 PM
nemanjai accepted this revision.Mar 23 2020, 4:46 AM

LGTM. Thanks for cleaning it up. I was mainly concerned with making sure this is an omission rather than something that was done on purpose to work around some bug. But it does indeed appear to be something we accidentally added.

This revision was automatically updated to reflect the committed changes.