This is an archive of the discontinued LLVM Phabricator instance.

Differential D154394

[PowerPC] Add DFP conversion instructions definitions and MC tests
ClosedPublic

Authored by lei on Jul 3 2023, 3:43 PM.

Details

Reviewers
nemanjai
stefanp
amyk
power-llvm-team
Commits
rGf20660701dff: [PowerPC] Add DFP conversion instructions definitions and MC tests
Summary

Add td definitions and asm/disasm tests for the quantum conversion
instructions in ISA 3.1 section 5.6.5

Diff Detail

Event Timeline

lei created this revision.Jul 3 2023, 3:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2023, 3:43 PM
Herald added subscribers: shchenz, hiraditya. · View Herald Transcript
lei requested review of this revision.Jul 3 2023, 3:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2023, 3:43 PM
Harbormaster completed remote builds in B242899: Diff 536910.Jul 3 2023, 5:23 PM
lei updated this revision to Diff 536938.Jul 3 2023, 7:56 PM

fix indentation.

Harbormaster completed remote builds in B242923: Diff 536938.Jul 3 2023, 9:21 PM
lei updated this revision to Diff 537073.Jul 4 2023, 6:00 AM

clang-format

Harbormaster completed remote builds in B243020: Diff 537073.Jul 4 2023, 6:51 AM
stefanp added a comment.Jul 4 2023, 11:12 AM

Mostly looks good just a handful of comments.

llvm/lib/Target/PowerPC/PPCInstrDFP.td
32

I feel like this should still be FRTB like the parent class.

130

nit:
The ins here should be (ins f8rc:$FRB).

140

I don't know if it's easier but you can do something like:

let XX = 1 in {
  def DCTFIXQQ: XForm_FRTB5<63, 994, (outs vrrc:$FRT), (ins fpairrc:$FRB),
                            "dctfixqq $FRT, $FRB", [], IIC_FPGeneral>;
} // XX = 1

And then you don't need the extra class at all. I'm not sure if that's easier / clearer in this situation.

lei updated this revision to Diff 537149.Jul 4 2023, 12:07 PM

remove new class def as there's an existing one.

lei updated this revision to Diff 537153.Jul 4 2023, 12:20 PM

address review comments

lei updated this revision to Diff 537159.Jul 4 2023, 12:44 PM

rebase to ToT

Harbormaster completed remote builds in B243076: Diff 537159.Jul 4 2023, 1:43 PM
lei updated this revision to Diff 537184.Jul 4 2023, 3:20 PM

update description

Harbormaster completed remote builds in B243098: Diff 537184.Jul 4 2023, 4:09 PM
stefanp accepted this revision.Jul 5 2023, 4:19 AM

Thank you for adding these instructions!
LGTM

This revision is now accepted and ready to land.Jul 5 2023, 4:19 AM
amyk added a comment.Jul 11 2023, 10:27 AM

Just a comment regarding indentation but otherwise I think LGTM, too.

llvm/lib/Target/PowerPC/PPCInstrDFP.td
94

Could we fix the indentation on the second lines of each instruction definition?

lei updated this revision to Diff 539614.Jul 12 2023, 9:53 AM
lei marked an inline comment as done.

fix indentation.

This revision was landed with ongoing or failed builds.Jul 12 2023, 10:57 AM
Closed by commit rGf20660701dff: [PowerPC] Add DFP conversion instructions definitions and MC tests (authored by lei). · Explain Why
This revision was automatically updated to reflect the committed changes.
lei added a commit: rGf20660701dff: [PowerPC] Add DFP conversion instructions definitions and MC tests.
Harbormaster completed remote builds in B244834: Diff 539614.Jul 12 2023, 1:52 PM

Revision Contents

PathSize
llvm/
lib/
Target/
PowerPC/
26 lines
1 line
test/
MC/
Disassembler/
PowerPC/
54 lines
PowerPC/
54 lines

Diff 539641

llvm/lib/Target/PowerPC/PPCInstrDFP.td

Show All 23 Lines
} }
defm DSUB : XForm_28r<59, 514, (outs f8rc:$RST), (ins f8rc:$RA, f8rc:$RB), defm DSUB : XForm_28r<59, 514, (outs f8rc:$RST), (ins f8rc:$RA, f8rc:$RB),
"dsub", "$RST, $RA, $RB", IIC_FPGeneral, []>; "dsub", "$RST, $RA, $RB", IIC_FPGeneral, []>;
defm DSUBQ : XForm_28r<63, 514, (outs fpairrc:$RST), (ins fpairrc:$RA, fpairrc:$RB), defm DSUBQ : XForm_28r<63, 514, (outs fpairrc:$RST), (ins fpairrc:$RA, fpairrc:$RB),
"dsubq", "$RST, $RA, $RB", IIC_FPGeneral, []>; "dsubq", "$RST, $RA, $RB", IIC_FPGeneral, []>;
let isCommutable = 1 in { let isCommutable = 1 in {
stefanpUnsubmitted
Not Done
ReplyInline Actions

I feel like this should still be FRTB like the parent class.

stefanp: I feel like this should still be `FRTB` like the parent class.
defm DMUL : XForm_28r<59, 34, (outs f8rc:$RST), (ins f8rc:$RA, f8rc:$RB), defm DMUL : XForm_28r<59, 34, (outs f8rc:$RST), (ins f8rc:$RA, f8rc:$RB),
"dmul", "$RST, $RA, $RB", IIC_FPGeneral, []>; "dmul", "$RST, $RA, $RB", IIC_FPGeneral, []>;
defm DMULQ : XForm_28r<63, 34, (outs fpairrc:$RST), (ins fpairrc:$RA, fpairrc:$RB), defm DMULQ : XForm_28r<63, 34, (outs fpairrc:$RST), (ins fpairrc:$RA, fpairrc:$RB),
"dmulq", "$RST, $RA, $RB", IIC_FPGeneral, []>; "dmulq", "$RST, $RA, $RB", IIC_FPGeneral, []>;
} }
defm DDIV : XForm_28r<59, 546, (outs f8rc:$RST), (ins f8rc:$RA, f8rc:$RB), defm DDIV : XForm_28r<59, 546, (outs f8rc:$RST), (ins f8rc:$RA, f8rc:$RB),
▲ Show 20 Lines • Show All 43 Lines • ▼ Show 20 Linesdefm DRINTXQ: Z23Form_FRTB5_R1_RMC2r<63, 99, (outs fpairrc:$FRT),
"drintxq", "$R, $FRT, $FRB, $RMC", []>; "drintxq", "$R, $FRT, $FRB, $RMC", []>;
defm DRINTN: Z23Form_FRTB5_R1_RMC2r<59, 227, (outs f8rc:$FRT), defm DRINTN: Z23Form_FRTB5_R1_RMC2r<59, 227, (outs f8rc:$FRT),
(ins u1imm:$R, f8rc:$FRB, u2imm:$RMC), (ins u1imm:$R, f8rc:$FRB, u2imm:$RMC),
"drintn", "$R, $FRT, $FRB, $RMC", []>; "drintn", "$R, $FRT, $FRB, $RMC", []>;
defm DRINTNQ: Z23Form_FRTB5_R1_RMC2r<63, 227, (outs fpairrc:$FRT), defm DRINTNQ: Z23Form_FRTB5_R1_RMC2r<63, 227, (outs fpairrc:$FRT),
(ins u1imm:$R, fpairrc:$FRB, u2imm:$RMC), (ins u1imm:$R, fpairrc:$FRB, u2imm:$RMC),
"drintnq", "$R, $FRT, $FRB, $RMC", []>; "drintnq", "$R, $FRT, $FRB, $RMC", []>;
// 5.6.5 DFP Conversion Instructions
defm DCTDP: XForm_26r<59, 258, (outs f8rc:$RST), (ins f8rc:$RB),
"dctdp", "$RST, $RB", IIC_FPGeneral, []>;
amykUnsubmitted
Done
ReplyInline Actions

Could we fix the indentation on the second lines of each instruction definition?

amyk: Could we fix the indentation on the second lines of each instruction definition?
defm DCTQPQ: XForm_26r<63, 258, (outs fpairrc:$RST), (ins f8rc:$RB),
"dctqpq", "$RST, $RB", IIC_FPGeneral, []>;
defm DRSP: XForm_26r<59, 770, (outs f8rc:$RST), (ins f8rc:$RB),
"drsp", "$RST, $RB", IIC_FPGeneral, []>;
defm DRDPQ: XForm_26r<63, 770, (outs fpairrc:$RST), (ins fpairrc:$RB),
"drdpq", "$RST, $RB", IIC_FPGeneral, []>;
defm DCFFIX: XForm_26r<59, 802, (outs f8rc:$RST), (ins f8rc:$RB),
"dcffix", "$RST, $RB", IIC_FPGeneral, []>;
defm DCFFIXQ: XForm_26r<63, 802, (outs fpairrc:$RST), (ins f8rc:$RB),
"dcffixq", "$RST, $RB", IIC_FPGeneral, []>;
defm DCTFIX: XForm_26r<59, 290, (outs f8rc:$RST), (ins f8rc:$RB),
"dctfix", "$RST, $RB", IIC_FPGeneral, []>;
defm DCTFIXQ: XForm_26r<63, 290, (outs f8rc:$RST), (ins fpairrc:$RB),
"dctfixq", "$RST, $RB", IIC_FPGeneral, []>;
let Predicates = [HasP10Vector] in {
def DCFFIXQQ: XForm_26<63, 994, (outs fpairrc:$RST), (ins vrrc:$RB),
"dcffixqq $RST, $RB", IIC_FPGeneral, []>;
let RA = 1 in
def DCTFIXQQ: XForm_base_r3xo<63, 994, (outs vrrc:$RST), (ins fpairrc:$RB),
"dctfixqq $RST, $RB", IIC_FPGeneral, []>;
} // HasP10Vector
} // mayRaiseFPException } // mayRaiseFPException
} // hasNoSchedulingInfo } // hasNoSchedulingInfo
stefanpUnsubmitted
Not Done
ReplyInline Actions

nit:
The ins here should be (ins f8rc:$FRB).

stefanp: nit: The `ins` here should be `(ins f8rc:$FRB)`.
stefanpUnsubmitted
Not Done
ReplyInline Actions

I don't know if it's easier but you can do something like:

let XX = 1 in {
  def DCTFIXQQ: XForm_FRTB5<63, 994, (outs vrrc:$FRT), (ins fpairrc:$FRB),
                            "dctfixqq $FRT, $FRB", [], IIC_FPGeneral>;
} // XX = 1

And then you don't need the extra class at all. I'm not sure if that's easier / clearer in this situation.

stefanp: I don't know if it's easier but you can do something like: ``` let XX = 1 in { def DCTFIXQQ…

llvm/lib/Target/PowerPC/PPCInstrVSX.td

Show First 20 Lines • Show All 123 Lines • ▼ Show 20 Lines
def HasOnlySwappingMemOps : Predicate<"!Subtarget->hasP9Vector()">; def HasOnlySwappingMemOps : Predicate<"!Subtarget->hasP9Vector()">;
def NoP8Vector : Predicate<"!Subtarget->hasP8Vector()">; def NoP8Vector : Predicate<"!Subtarget->hasP8Vector()">;
def HasP8Vector : Predicate<"Subtarget->hasP8Vector()">; def HasP8Vector : Predicate<"Subtarget->hasP8Vector()">;
def HasDirectMove : Predicate<"Subtarget->hasDirectMove()">; def HasDirectMove : Predicate<"Subtarget->hasDirectMove()">;
def NoP9Vector : Predicate<"!Subtarget->hasP9Vector()">; def NoP9Vector : Predicate<"!Subtarget->hasP9Vector()">;
def HasP9Vector : Predicate<"Subtarget->hasP9Vector()">; def HasP9Vector : Predicate<"Subtarget->hasP9Vector()">;
def NoP9Altivec : Predicate<"!Subtarget->hasP9Altivec()">; def NoP9Altivec : Predicate<"!Subtarget->hasP9Altivec()">;
def NoP10Vector: Predicate<"!Subtarget->hasP10Vector()">; def NoP10Vector: Predicate<"!Subtarget->hasP10Vector()">;
def HasP10Vector: Predicate<"Subtarget->hasP10Vector()">;
def PPCldsplatAlign16 : PatFrag<(ops node:$ptr), (PPCldsplat node:$ptr), [{ def PPCldsplatAlign16 : PatFrag<(ops node:$ptr), (PPCldsplat node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getAlign() >= Align(16) && return cast<MemIntrinsicSDNode>(N)->getAlign() >= Align(16) &&
isOffsetMultipleOf(N, 16); isOffsetMultipleOf(N, 16);
}]>; }]>;
//--------------------- VSX-specific instruction formats ---------------------// //--------------------- VSX-specific instruction formats ---------------------//
// By default, all VSX instructions are to be selected over their Altivec // By default, all VSX instructions are to be selected over their Altivec
▲ Show 20 Lines • Show All 5,012 Lines • Show Last 20 Lines

llvm/test/MC/Disassembler/PowerPC/ppc64-encoding-dfp.txt

Show First 20 Lines • Show All 113 Lines • ▼ Show 20 Lines
# CHECK: drintn. 1, 10, 6, 2 # CHECK: drintn. 1, 10, 6, 2
0xed 0x41 0x35 0xc7 0xed 0x41 0x35 0xc7
# CHECK: drintnq 1, 10, 6, 2 # CHECK: drintnq 1, 10, 6, 2
0xfd 0x41 0x35 0xc6 0xfd 0x41 0x35 0xc6
# CHECK: drintnq. 0, 10, 6, 2 # CHECK: drintnq. 0, 10, 6, 2
0xfd 0x40 0x35 0xc7 0xfd 0x40 0x35 0xc7
# CHECK: dctdp 8, 2
0xed 0x00 0x12 0x04
# CHECK: dctdp. 8, 2
0xed 0x00 0x12 0x05
# CHECK: dctqpq 8, 2
0xfd 0x00 0x12 0x04
# CHECK: dctqpq. 8, 2
0xfd 0x00 0x12 0x05
# CHECK: drsp 20, 8
0xee 0x80 0x46 0x04
# CHECK: drsp. 20, 8
0xee 0x80 0x46 0x05
# CHECK: drdpq 20, 8
0xfe 0x80 0x46 0x04
# CHECK: drdpq. 20, 8
0xfe 0x80 0x46 0x05
# CHECK: dcffix 12, 7
0xed 0x80 0x3e 0x44
# CHECK: dcffix. 12, 7
0xed 0x80 0x3e 0x45
# CHECK: dcffixq 12, 8
0xfd 0x80 0x46 0x44
# CHECK: dcffixq. 12, 8
0xfd 0x80 0x46 0x45
# CHECK : dcffixqq 18, 20
0xfe 0x40 0xa7 0xc4
# CHECK: dctfix 8, 4
0xed 0x00 0x22 0x44
# CHECK: dctfix. 8, 4
0xed 0x00 0x22 0x45
# CHECK: dctfixq 8, 4
0xfd 0x00 0x22 0x44
# CHECK: dctfixq. 8, 4
0xfd 0x00 0x22 0x45
# CHECK: dctfixqq 8, 10
0xfd 0x01 0x57 0xc4

llvm/test/MC/PowerPC/ppc64-encoding-dfp.s

Show First 20 Lines • Show All 116 Lines • ▼ Show 20 Lines
# CHECK-BE: drintn. 0, 10, 6, 2 # encoding: [0xed,0x40,0x35,0xc7] # CHECK-BE: drintn. 0, 10, 6, 2 # encoding: [0xed,0x40,0x35,0xc7]
drintn. 0, 10, 6, 2 drintn. 0, 10, 6, 2
# CHECK-LE: drintnq 0, 10, 6, 2 # encoding: [0xc6,0x35,0x40,0xfd] # CHECK-LE: drintnq 0, 10, 6, 2 # encoding: [0xc6,0x35,0x40,0xfd]
# CHECK-BE: drintnq 0, 10, 6, 2 # encoding: [0xfd,0x40,0x35,0xc6] # CHECK-BE: drintnq 0, 10, 6, 2 # encoding: [0xfd,0x40,0x35,0xc6]
drintnq 0, 10, 6, 2 drintnq 0, 10, 6, 2
# CHECK-LE: drintnq. 1, 10, 6, 2 # encoding: [0xc7,0x35,0x41,0xfd] # CHECK-LE: drintnq. 1, 10, 6, 2 # encoding: [0xc7,0x35,0x41,0xfd]
# CHECK-BE: drintnq. 1, 10, 6, 2 # encoding: [0xfd,0x41,0x35,0xc7] # CHECK-BE: drintnq. 1, 10, 6, 2 # encoding: [0xfd,0x41,0x35,0xc7]
drintnq. 1, 10, 6, 2 drintnq. 1, 10, 6, 2
# CHECK-LE: dctdp 8, 2 # encoding: [0x04,0x12,0x00,0xed]
# CHECK-BE: dctdp 8, 2 # encoding: [0xed,0x00,0x12,0x04]
dctdp 8, 2
# CHECK-LE: dctdp. 8, 2 # encoding: [0x05,0x12,0x00,0xed]
# CHECK-BE: dctdp. 8, 2 # encoding: [0xed,0x00,0x12,0x05]
dctdp. 8, 2
# CHECK-LE: dctqpq 8, 2 # encoding: [0x04,0x12,0x00,0xfd]
# CHECK-BE: dctqpq 8, 2 # encoding: [0xfd,0x00,0x12,0x04]
dctqpq 8, 2
# CHECK-LE: dctqpq. 8, 2 # encoding: [0x05,0x12,0x00,0xfd]
# CHECK-BE: dctqpq. 8, 2 # encoding: [0xfd,0x00,0x12,0x05]
dctqpq. 8, 2
# CHECK-LE: drsp 20, 8 # encoding: [0x04,0x46,0x80,0xee]
# CHECK-BE: drsp 20, 8 # encoding: [0xee,0x80,0x46,0x04]
drsp 20, 8
# CHECK-LE: drsp. 20, 8 # encoding: [0x05,0x46,0x80,0xee]
# CHECK-BE: drsp. 20, 8 # encoding: [0xee,0x80,0x46,0x05]
drsp. 20, 8
# CHECK-LE: drdpq 20, 8 # encoding: [0x04,0x46,0x80,0xfe]
# CHECK-BE: drdpq 20, 8 # encoding: [0xfe,0x80,0x46,0x04]
drdpq 20, 8
# CHECK-LE: drdpq. 20, 8 # encoding: [0x05,0x46,0x80,0xfe]
# CHECK-BE: drdpq. 20, 8 # encoding: [0xfe,0x80,0x46,0x05]
drdpq. 20, 8
# CHECK-LE: dcffix 12, 7 # encoding: [0x44,0x3e,0x80,0xed]
# CHECK-BE: dcffix 12, 7 # encoding: [0xed,0x80,0x3e,0x44]
dcffix 12, 7
# CHECK-LE: dcffix. 12, 7 # encoding: [0x45,0x3e,0x80,0xed]
# CHECK-BE: dcffix. 12, 7 # encoding: [0xed,0x80,0x3e,0x45]
dcffix. 12, 7
# CHECK-LE: dcffixq 12, 8 # encoding: [0x44,0x46,0x80,0xfd]
# CHECK-BE: dcffixq 12, 8 # encoding: [0xfd,0x80,0x46,0x44]
dcffixq 12, 8
# CHECK-LE: dcffixq. 12, 8 # encoding: [0x45,0x46,0x80,0xfd]
# CHECK-BE: dcffixq. 12, 8 # encoding: [0xfd,0x80,0x46,0x45]
dcffixq. 12, 8
# CHECK-LE: dctfix 8, 4 # encoding: [0x44,0x22,0x00,0xed]
# CHECK-BE: dctfix 8, 4 # encoding: [0xed,0x00,0x22,0x44]
dctfix 8, 4
# CHECK-LE: dctfix. 8, 4 # encoding: [0x45,0x22,0x00,0xed]
# CHECK-BE: dctfix. 8, 4 # encoding: [0xed,0x00,0x22,0x45]
dctfix. 8, 4
# CHECK-LE: dctfixq 8, 4 # encoding: [0x44,0x22,0x00,0xfd]
# CHECK-BE: dctfixq 8, 4 # encoding: [0xfd,0x00,0x22,0x44]
dctfixq 8, 4
# CHECK-LE: dctfixq. 8, 4 # encoding: [0x45,0x22,0x00,0xfd]
# CHECK-BE: dctfixq. 8, 4 # encoding: [0xfd,0x00,0x22,0x45]
dctfixq. 8, 4
# CHECK-LE: dcffixqq 18, 20 # encoding: [0xc4,0xa7,0x40,0xfe]
# CHECK-BE: dcffixqq 18, 20 # encoding: [0xfe,0x40,0xa7,0xc4]
dcffixqq 18, 20
# CHECK-LE: dctfixqq 8, 10 # encoding: [0xc4,0x57,0x01,0xfd]
# CHECK-BE: dctfixqq 8, 10 # encoding: [0xfd,0x01,0x57,0xc4]
dctfixqq 8, 10