This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC][Altivec] Add mfvrd and mffprd extended mnemonic
ClosedPublic

Authored by nemanjai on Jan 26 2017, 6:33 AM.

Details

Summary

mfvrd and mffprd are both alias to mfvrsd.
This patch enables correct parsing of the aliases, but we still emit a mfvrsd.

Patch by nemanjai

Diff Detail

Repository
rL LLVM

Event Timeline

brunoalr created this revision.Jan 26 2017, 6:33 AM
hfinkel edited edge metadata.Jan 26 2017, 6:47 AM

Please add a test case so I can see what's going on.

Please add a test case so I can see what's going on.

@nemanjai can you help me with that?

Please add a test case so I can see what's going on.

Please add a test case so I can see what's going on.

@nemanjai can you help me with that?

I have a small test case written in C that is just a one-line asm statement, which currently yields an "unrecognized instruction mnemonic" error:

asm ("mfvrd 3, 10\n\t");

, but I lack the knowledge to convert it down to .ll .

nemanjai edited edge metadata.Jan 26 2017, 8:03 AM

Please add a test case so I can see what's going on.

Here's an example of what this will do now (as opposed to emitting the error that Bruno posted):

$ printf "mfvsrd 4, 34\nmfvrd 4, 2" | llvm-mc --show-encoding
      .text
      mfvsrd 4, 34                    # encoding: [0x67,0x00,0x44,0x7c]
      mfvsrd 4, 34                    # encoding: [0x67,0x00,0x44,0x7c]

Please add a test case so I can see what's going on.

Here's an example of what this will do now (as opposed to emitting the error that Bruno posted):

$ printf "mfvsrd 4, 34\nmfvrd 4, 2" | llvm-mc --show-encoding
      .text
      mfvsrd 4, 34                    # encoding: [0x67,0x00,0x44,0x7c]
      mfvsrd 4, 34                    # encoding: [0x67,0x00,0x44,0x7c]

Seems reasonable to me.

We are looking into the most adequate way of including these alias. But we still need extended mnemonics for the lower half of the VSX regs as well.

nemanjai commandeered this revision.Jan 31 2017, 9:51 PM
nemanjai edited reviewers, added: brunoalr; removed: nemanjai.

Sorry Bruno, it appears that I can only update this revision if I commandeer it.

nemanjai updated this revision to Diff 86576.Jan 31 2017, 9:54 PM

Provide both of the extended mnemonics for MFVSRD and add/update tests for them.

Please note that changes to test case p8-scalar_vector_conversions.ll may seem unrelated, but the unoptimized IR (no mem2reg) produces different mnemonics in some cases and not others (i.e. depending on which source register is selected). This update just locks the parameter registers to what the calling convention says and thereby fixes the mnemonic choice.

brunoalr retitled this revision from [PowerPC][Altivec] Add mfvrd extended mnemonic to [PowerPC][Altivec] Add mfvrd and mffprd extended mnemonic.Feb 1 2017, 4:00 AM
brunoalr edited the summary of this revision. (Show Details)
brunoalr edited edge metadata.

Sorry Bruno, it appears that I can only update this revision if I commandeer it.

No problem! It was you who wrote this patch after all :P
I updated the rev text to include mffprd as well.

Provide both of the extended mnemonics for MFVSRD and add/update tests for them.

Please note that changes to test case p8-scalar_vector_conversions.ll may seem unrelated, but the unoptimized IR (no mem2reg) produces different mnemonics in some cases and not others (i.e. depending on which source register is selected). This update just locks the parameter registers to what the calling convention says and thereby fixes the mnemonic choice.

Hi there, @nemanjai!
Is there any other action that I should take for this patch to get approved?

Thanks.

Hi there, @nemanjai!
Is there any other action that I should take for this patch to get approved?

Thanks.

@hfinkel @kbarton Can you guys have a look so we can get this committed?

hfinkel accepted this revision.Mar 13 2017, 2:02 PM

LGTM

This revision is now accepted and ready to land.Mar 13 2017, 2:02 PM

Hi there, @nemanjai!
Is there any other action that I should take for this patch to get approved?

Thanks.

@hfinkel @kbarton Can you guys have a look so we can get this committed?

thanks, @nemanjai, @hfinkel !

This revision was automatically updated to reflect the committed changes.