[AArch64] fjcvtzs,rmif,cfinv,setf* all clobber nzcv

[jroelofs]

Summary

Adding these defs will prevent these instructions from being scheduled within an nzcv live range.

[jroelofs]

@SjoerdMeijer mind double-checking me on this?

[SjoerdMeijer]

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.

[jroelofs]

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.

[SjoerdMeijer]

llvm/test/CodeGen/AArch64/fjcvtzs.ll

2

Testing this is probably a bit convenient, but I was thinking about a MIR test indeed, that should display all the implicit defs/uses.
This almost looks like a mir test, but doing it here in this way with llc and -stop-after is probably unconventional, see

https://llvm.org/docs/MIRLangRef.html#testing-individual-code-generation-passes

for creating a .mir test.

[SjoerdMeijer]

llvm/test/CodeGen/AArch64/fjcvtzs.ll

2

"Testing this is probably a bit convenient"
->
inconvenient

[jroelofs]

Add mir tests for each of the affected instructions.

[SjoerdMeijer]

llvm/test/CodeGen/AArch64/fjcvtzs.mir

1 ↗

(On Diff #280467)

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

[jroelofs]

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.

[SjoerdMeijer]

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.

[jroelofs]

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.

[jroelofs]

llvm/test/CodeGen/AArch64/fjcvtzs.mir

1 ↗

(On Diff #280467)

No worries. It's good for me to learn new tools!

[jroelofs]

rm subsumed test

[jroelofs]

Kinda verbose, but it works. This would be so much easier if we had a way to dump mir from llvm-mc.

[SjoerdMeijer]

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.

[jroelofs]

In D83818#2173227, @SjoerdMeijer wrote:

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 :(

llvm/lib/Target/AArch64/AArch64InstrFormats.td

   let Inst{4-0}   = Rd;
 }
 
 // Base class for the Armv8.4-A 8 and 16-bit flag manipulation instructions
 class BaseFlagManipulation<bit sf, bit sz, dag iops, string asm, string ops>
     : I<(outs), iops, asm, ops, "", []>,
       Sched<[WriteI, ReadI, ReadI]> {
   let Uses = [NZCV];
+  let Defs = [NZCV];
   bits<5> Rn;
   let Inst{31}    = sf;
   let Inst{30-15} = 0b0111010000000000;
   let Inst{14}    = sz;
   let Inst{13-10} = 0b0010;
   let Inst{9-5}   = Rn;
   let Inst{4-0}   = 0b01101;
 }

llvm/lib/Target/AArch64/AArch64InstrInfo.td

   }
 
   defm LDRAA  : AuthLoad<0, "ldraa", simm10Scaled>;
   defm LDRAB  : AuthLoad<1, "ldrab", simm10Scaled>;
 
 }
 
 // v8.3a floating point conversion for javascript
-let Predicates = [HasJS, HasFPARMv8] in
+let Predicates = [HasJS, HasFPARMv8], Defs = [NZCV] in
 def FJCVTZS  : BaseFPToIntegerUnscaled<0b01, 0b11, 0b110, FPR64, GPR32,
                                       "fjcvtzs",
                                       [(set GPR32:$Rd,
                                          (int_aarch64_fjcvtzs FPR64:$Rn))]> {
   let Inst{31} = 0;
 } // HasJS, HasFPARMv8
 
 // v8.4 Flag manipulation instructions
-let Predicates = [HasFMI] in {
+let Predicates = [HasFMI], Defs = [NZCV], Uses = [NZCV] in {
 def CFINV : SimpleSystemI<0, (ins), "cfinv", "">, Sched<[WriteSys]> {
   let Inst{20-5} = 0b0000001000000000;
 }
 def SETF8  : BaseFlagManipulation<0, 0, (ins GPR32:$Rn), "setf8", "{\t$Rn}">;
 def SETF16 : BaseFlagManipulation<0, 1, (ins GPR32:$Rn), "setf16", "{\t$Rn}">;
 def RMIF   : FlagRotate<(ins GPR64:$Rn, uimm6:$imm, imm0_15:$mask), "rmif",
                         "{\t$Rn, $imm, $mask}">;
 } // HasFMI

llvm/test/CodeGen/AArch64/fjcvtzs.ll

 ; RUN: llc -mtriple=arm64-eabi -mattr=+jsconv -o - %s | FileCheck %s
+; RUN: llc -mtriple=arm64-eabi -mattr=+jsconv -stop-after=legalizer -o - %s | FileCheck %s --check-prefix=CHECK-MIR
 
 define i32 @test_jcvt(double %v) {
 ; CHECK-LABEL: test_jcvt:
 ; CHECK: fjcvtzs w0, d0
+; CHECK-MIR: FJCVTZS
+; CHECK-MIR-SAME: implicit-def dead $nzcv
   %val = call i32 @llvm.aarch64.fjcvtzs(double %v)
   ret i32 %val
 }
 
 declare i32 @llvm.aarch64.fjcvtzs(double)