This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC-QPX] adjust operands order of qpx vector fma instructions.
ClosedPublic

Authored by shchenz on Apr 28 2020, 1:54 AM.

Details

Summary

Now after isel, operand order for qpx vector fma instruction qvfmadd is like:

%3 = QVFMADD %2, %0, %1, implicit $rm

This stands for %3 = %2 * %1 + %0. This is a little different with QPX ISA's description and also not same with other Power arch vector fma instrucions, like xvmaddadp

It should be like %3 = QVFMADD %2, %1, %0, implicit $rm

Diff Detail

Event Timeline

shchenz created this revision.Apr 28 2020, 1:54 AM
steven.zhang added inline comments.
llvm/lib/Target/PowerPC/PPCInstrQPX.td
179

So what about the QVFNMADD ? And it would be great if you could post the ISA definition of QPX FMA instruction here.

shchenz updated this revision to Diff 260837.Apr 28 2020, 10:22 PM

add more opcodes and test cases

shchenz marked 2 inline comments as done.Apr 28 2020, 10:25 PM

Thanks for your quick review. Patch updated.

llvm/lib/Target/PowerPC/PPCInstrQPX.td
179

QVFNMADD should have same issue. I already add it together with msub related opcodes.

Here is QPX ISA (May 9, 2012) says about QVFMADD:

4.4.2 Quad-Vector Floating-Point Multiply-Add Instructions

qvfmadds QRT,QRA,QRC,QRB

The operations are performed:
QRT0 ← [(QRA0)×(QRC0)] + (QRB0) 
QRT1 ← [(QRA1)×(QRC1)] + (QRB1) 
QRT2 ← [(QRA2)×(QRC2)] + (QRB2) 
QRT3 ← [(QRA3)×(QRC3)] + (QRB3)
steven.zhang added inline comments.
llvm/lib/Target/PowerPC/PPCInstrQPX.td
902

This is not right as we didn't respect the nsz flag here. But it is not relative with this patch. D76585 miss to handle the QPX. @qiucf

hfinkel accepted this revision as: hfinkel.Apr 29 2020, 9:39 AM

This LGTM for consistency. Also, it reminds me that it's probably time for an RFC to remove QPX support in general.

This revision is now accepted and ready to land.Apr 29 2020, 9:39 AM
jsji added a comment.Apr 29 2020, 9:49 AM

Good suggestion. Will you send the RFC? Or you would like us to send it? Thanks. @hfinkel

shchenz marked an inline comment as done.Apr 29 2020, 8:22 PM

Thanks for reviewing. @hfinkel . Since you already sent RFC about removing the QPX support, I will hold this patch until we get the decision.

I will commit this first as I have one patch based on this.
if we start to remove qpx support in future, this should be easy to delete.

This revision was automatically updated to reflect the committed changes.