Adding these defs will prevent these instructions from being scheduled within an nzcv live range.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Agreed. From the ArmARM:
Floating-point FJCVTZS instruction sets the PSTATE.Z flag if the result of the conversion, when converted back to a double-precision floating-point number, gives precisely the same value as the original. Other PSTATE flags are cleared by this instruction
And there are similar descriptions for the other instructions.
With the instruction descriptions fixed, would it be possible to add a test case for this:
Adding these defs will prevent these instructions from being scheduled within an nzcv live range.
Add a test for the changes for fjcvtzs.
@SjoerdMeijer can you suggest a good way to write a test for the others? llvm-mc doesn't seem to have a way to print implicit defs/uses.
llvm/test/CodeGen/AArch64/fjcvtzs.ll | ||
---|---|---|
2 ↗ | (On Diff #280232) | Testing this is probably a bit convenient, but I was thinking about a MIR test indeed, that should display all the implicit defs/uses. https://llvm.org/docs/MIRLangRef.html#testing-individual-code-generation-passes for creating a .mir test. |
llvm/test/CodeGen/AArch64/fjcvtzs.ll | ||
---|---|---|
2 ↗ | (On Diff #280232) | "Testing this is probably a bit convenient" |
llvm/test/CodeGen/AArch64/fjcvtzs.mir | ||
---|---|---|
2 | I am very sorry, looking at this, I am changing my mind on this. This test is not testing anything, there are no CHECK tags. And in fact, what you had before is better and what we want: an llc test fed with an .ll, using stop-after to produce mir and check for implicit defs/uses. | |
llvm/test/CodeGen/AArch64/flag-mainpulation.mir | ||
1 ↗ | (On Diff #280467) | nit: typo in the file name: flag-mainpulation.mir |
llvm/lib/Target/AArch64/AArch64InstrInfo.td | ||
---|---|---|
1042 | Disappointingly, these new tests don't fail if I remove these Defs/Uses, so they're not really effective at enforcing that these be here. The "unconventional" test I had before did that correctly, but I'm not sure how to write one for this group of instructions, since they're not matched by any PatFrag, and don't have corresponding intrinsics. |
llvm/lib/Target/AArch64/AArch64InstrInfo.td | ||
---|---|---|
1042 | Okay, thanks for trying. I think we've really tried, a change without tests here seems reasonable then. |
llvm/lib/Target/AArch64/AArch64InstrInfo.td | ||
---|---|---|
1042 | Ooh, it just occurred to me that there is a way to test this: FileCheck for the error messages with mir that doesn't compile because the uses/defs are missing. |
llvm/test/CodeGen/AArch64/fjcvtzs.mir | ||
---|---|---|
2 | No worries. It's good for me to learn new tools! |
Kinda verbose, but it works. This would be so much easier if we had a way to dump mir from llvm-mc.
Nice one! LGTM
Optional: since they are really small tests, you can try if it is convenient to have them all in one file, but separate is fine too I think.
I wanted that, but couldn't find a way to make it work. The MIRParser quits after the first error :(
Disappointingly, these new tests don't fail if I remove these Defs/Uses, so they're not really effective at enforcing that these be here. The "unconventional" test I had before did that correctly, but I'm not sure how to write one for this group of instructions, since they're not matched by any PatFrag, and don't have corresponding intrinsics.