[LoongArch] Add back SDNPSideEffect properties to CSR and IOCSR read ops

[xen0n]

Summary

In general, CSR and IOCSR reads should be treated as volatile because:

• there may well be intervening writes between seemingly common expressions;
• the stateful entity behind a given (IO)CSR may well be volatile.

Confirmed to fix broken Clang Linux/LoongArch builds (dying when a
userspace process tries to use FPU, panicking when that process happens
to be PID 1) with this patch.

Fixes: https://github.com/llvm/llvm-project/issues/63549
Fixes: 2efdacf74c54 ("[LoongArch] Add missing chains and remove unnecessary SDNPSideEffect property for some intrinsic nodes")

[xry111]

But it's puzzling. Is reading a CSR really a side effect?

[xen0n]

In D153865#4452222, @xry111 wrote:

But it's puzzling. Is reading a CSR really a side effect?

I didn't look deeper regarding how to best model this (effectively I'm just reverting some changes), but the crux is that (IO)CSR reads should in general be treated as volatile.

[xry111]

Ah, in the C standard "reading a volatile variable" is indeed a side effect. I must have been thinking about another definition of "side effect".

But should we do it for movfcsr2gr as well? It cannot be reordered with any floating-point arithmetic instructions.

[SixWeining]

Interesting. Does cpucfg have the same issue?

[xry111]

In D153865#4452321, @SixWeining wrote:

Interesting. Does cpucfg have the same issue?

As at now all cpucfg info seem constant.

[xen0n]

In D153865#4452321, @SixWeining wrote:

Interesting. Does cpucfg have the same issue?

In theory CPUCFG data could be altered by writing to certain model-specific CSRs, but I don't think it's worthwhile to make it volatile too, because no kernel should be doing that under the hood (at most do this once before starting even PID1; in fact no known LoongArch OSes do this).

[xen0n]

In D153865#4452309, @xry111 wrote:

Ah, in the C standard "reading a volatile variable" is indeed a side effect. I must have been thinking about another definition of "side effect".

But should we do it for movfcsr2gr as well? It cannot be reordered with any floating-point arithmetic instructions.

Ah indeed. There are Flags and Cause fields that are effectively, flag bits. FCSR reads should be free of side-effects but reordering indeed should get inhibited, I think it's probably better to deal with that in a separate patch.

[xen0n]

In D153865#4452333, @xen0n wrote:

In D153865#4452321, @SixWeining wrote:

Interesting. Does cpucfg have the same issue?

In theory CPUCFG data could be altered by writing to certain model-specific CSRs, but I don't think it's worthwhile to make it volatile too, because no kernel should be doing that under the hood (at most do this once before starting even PID1; in fact no known LoongArch OSes do this).

Another reason why changing CPUCFG is effectively never done: the MCSRs are entirely undocumented (the only existence being in the kernel sources and semantics only narrowly and anecdotally dispersed), thus unreliable to depend on.

[xen0n]

Update some missed test fixtures.

[MaskRay]

LGTM. This patch prevents DeadMachineInstructionElim from removing the instructions with unmodeled side effects.

llvm/lib/Target/LoongArch/LoongArchInstrInfo.td

 def loongarch_movfcsr2gr : SDNode<"LoongArchISD::MOVFCSR2GR",
                                   SDT_LoongArchMovfcsr2gr, [SDNPHasChain]>;
 def loongarch_movgr2fcsr : SDNode<"LoongArchISD::MOVGR2FCSR",
                                   SDT_LoongArchMovgr2fcsr,
                                   [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_syscall : SDNode<"LoongArchISD::SYSCALL", SDT_LoongArchVI,
                                 [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_csrrd : SDNode<"LoongArchISD::CSRRD", SDT_LoongArchCsrrd,
-                              [SDNPHasChain]>;
+                              [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_csrwr : SDNode<"LoongArchISD::CSRWR", SDT_LoongArchCsrwr,
                               [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_csrxchg : SDNode<"LoongArchISD::CSRXCHG",
                                 SDT_LoongArchCsrxchg,
                                 [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_iocsrrd_b : SDNode<"LoongArchISD::IOCSRRD_B", SDTUnaryOp,
-                                  [SDNPHasChain]>;
+                                  [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_iocsrrd_h : SDNode<"LoongArchISD::IOCSRRD_H", SDTUnaryOp,
-                                  [SDNPHasChain]>;
+                                  [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_iocsrrd_w : SDNode<"LoongArchISD::IOCSRRD_W", SDTUnaryOp,
-                                  [SDNPHasChain]>;
+                                  [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_iocsrrd_d : SDNode<"LoongArchISD::IOCSRRD_D", SDTUnaryOp,
-                                  [SDNPHasChain]>;
+                                  [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_iocsrwr_b : SDNode<"LoongArchISD::IOCSRWR_B",
                                   SDT_LoongArchIocsrwr,
                                   [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_iocsrwr_h : SDNode<"LoongArchISD::IOCSRWR_H",
                                   SDT_LoongArchIocsrwr,
                                   [SDNPHasChain, SDNPSideEffect]>;
 def loongarch_iocsrwr_w : SDNode<"LoongArchISD::IOCSRWR_W",
                                   SDT_LoongArchIocsrwr,

llvm/test/CodeGen/LoongArch/intrinsic-csr-side-effects.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc --mtriple=loongarch32 < %s | FileCheck %s
+; RUN: llc --mtriple=loongarch64 < %s | FileCheck %s
+
+declare i32 @llvm.loongarch.csrrd.w(i32 immarg) nounwind
+declare i32 @llvm.loongarch.csrwr.w(i32, i32 immarg) nounwind
+declare void @bug()
+
+define dso_local void @foo(i32 noundef signext %flag) nounwind {
+; CHECK-LABEL: foo:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    beqz $a0, .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %if.then
+; CHECK-NEXT:    csrrd $a0, 2
+; CHECK-NEXT:    ori $a0, $a0, 1
+; CHECK-NEXT:    csrwr $a0, 2
+; CHECK-NEXT:  .LBB0_2: # %if.end
+; CHECK-NEXT:    csrrd $a0, 2
+; CHECK-NEXT:    andi $a0, $a0, 1
+; CHECK-NEXT:    bnez $a0, .LBB0_4
+; CHECK-NEXT:  # %bb.3: # %if.then2
+; CHECK-NEXT:    b %plt(bug)
+; CHECK-NEXT:  .LBB0_4: # %if.end3
+; CHECK-NEXT:    ret
+entry:
+  %tobool.not = icmp eq i32 %flag, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = tail call i32 @llvm.loongarch.csrrd.w(i32 2)
+  %or = or i32 %0, 1
+  %1 = tail call i32 @llvm.loongarch.csrwr.w(i32 %or, i32 2)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  %2 = tail call i32 @llvm.loongarch.csrrd.w(i32 2)
+  %and = and i32 %2, 1
+  %tobool1.not = icmp eq i32 %and, 0
+  br i1 %tobool1.not, label %if.then2, label %if.end3
+
+if.then2:                                         ; preds = %if.end
+  tail call void @bug()
+  br label %if.end3
+
+if.end3:                                          ; preds = %if.then2, %if.end
+  ret void
+}

llvm/test/CodeGen/LoongArch/intrinsic-iocsr-side-effects.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc --mtriple=loongarch64 < %s | FileCheck %s
+
+declare i32 @llvm.loongarch.iocsrrd.b(i32) nounwind
+declare void @llvm.loongarch.iocsrwr.b(i32, i32) nounwind
+declare i32 @llvm.loongarch.iocsrrd.h(i32) nounwind
+declare void @llvm.loongarch.iocsrwr.h(i32, i32) nounwind
+declare i32 @llvm.loongarch.iocsrrd.w(i32) nounwind
+declare void @llvm.loongarch.iocsrwr.w(i32, i32) nounwind
+declare i64 @llvm.loongarch.iocsrrd.d(i32) nounwind
+declare void @llvm.loongarch.iocsrwr.d(i64, i32) nounwind
+declare void @bug()
+
+define dso_local void @test_b(i32 noundef signext %flag) nounwind {
+; CHECK-LABEL: test_b:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    beqz $a0, .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %if.then
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.b $a1, $a0
+; CHECK-NEXT:    ori $a1, $a1, 1
+; CHECK-NEXT:    iocsrwr.b $a1, $a0
+; CHECK-NEXT:  .LBB0_2: # %if.end
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.b $a0, $a0
+; CHECK-NEXT:    andi $a0, $a0, 1
+; CHECK-NEXT:    bnez $a0, .LBB0_4
+; CHECK-NEXT:  # %bb.3: # %if.then2
+; CHECK-NEXT:    b %plt(bug)
+; CHECK-NEXT:  .LBB0_4: # %if.end3
+; CHECK-NEXT:    ret
+entry:
+  %tobool.not = icmp eq i32 %flag, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = tail call i32 @llvm.loongarch.iocsrrd.b(i32 2)
+  %or = or i32 %0, 1
+  tail call void @llvm.loongarch.iocsrwr.b(i32 %or, i32 2)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  %1 = tail call i32 @llvm.loongarch.iocsrrd.b(i32 2)
+  %and = and i32 %1, 1
+  %tobool1.not = icmp eq i32 %and, 0
+  br i1 %tobool1.not, label %if.then2, label %if.end3
+
+if.then2:                                         ; preds = %if.end
+  tail call void @bug()
+  br label %if.end3
+
+if.end3:                                          ; preds = %if.then2, %if.end
+  ret void
+}
+
+define dso_local void @test_h(i32 noundef signext %flag) nounwind {
+; CHECK-LABEL: test_h:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    beqz $a0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %if.then
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.h $a1, $a0
+; CHECK-NEXT:    ori $a1, $a1, 1
+; CHECK-NEXT:    iocsrwr.h $a1, $a0
+; CHECK-NEXT:  .LBB1_2: # %if.end
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.h $a0, $a0
+; CHECK-NEXT:    andi $a0, $a0, 1
+; CHECK-NEXT:    bnez $a0, .LBB1_4
+; CHECK-NEXT:  # %bb.3: # %if.then2
+; CHECK-NEXT:    b %plt(bug)
+; CHECK-NEXT:  .LBB1_4: # %if.end3
+; CHECK-NEXT:    ret
+entry:
+  %tobool.not = icmp eq i32 %flag, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = tail call i32 @llvm.loongarch.iocsrrd.h(i32 2)
+  %or = or i32 %0, 1
+  tail call void @llvm.loongarch.iocsrwr.h(i32 %or, i32 2)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  %1 = tail call i32 @llvm.loongarch.iocsrrd.h(i32 2)
+  %and = and i32 %1, 1
+  %tobool1.not = icmp eq i32 %and, 0
+  br i1 %tobool1.not, label %if.then2, label %if.end3
+
+if.then2:                                         ; preds = %if.end
+  tail call void @bug()
+  br label %if.end3
+
+if.end3:                                          ; preds = %if.then2, %if.end
+  ret void
+}
+
+define dso_local void @test_w(i32 noundef signext %flag) nounwind {
+; CHECK-LABEL: test_w:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    beqz $a0, .LBB2_2
+; CHECK-NEXT:  # %bb.1: # %if.then
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.w $a1, $a0
+; CHECK-NEXT:    ori $a1, $a1, 1
+; CHECK-NEXT:    iocsrwr.w $a1, $a0
+; CHECK-NEXT:  .LBB2_2: # %if.end
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.w $a0, $a0
+; CHECK-NEXT:    andi $a0, $a0, 1
+; CHECK-NEXT:    bnez $a0, .LBB2_4
+; CHECK-NEXT:  # %bb.3: # %if.then2
+; CHECK-NEXT:    b %plt(bug)
+; CHECK-NEXT:  .LBB2_4: # %if.end3
+; CHECK-NEXT:    ret
+entry:
+  %tobool.not = icmp eq i32 %flag, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = tail call i32 @llvm.loongarch.iocsrrd.w(i32 2)
+  %or = or i32 %0, 1
+  tail call void @llvm.loongarch.iocsrwr.w(i32 %or, i32 2)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  %1 = tail call i32 @llvm.loongarch.iocsrrd.w(i32 2)
+  %and = and i32 %1, 1
+  %tobool1.not = icmp eq i32 %and, 0
+  br i1 %tobool1.not, label %if.then2, label %if.end3
+
+if.then2:                                         ; preds = %if.end
+  tail call void @bug()
+  br label %if.end3
+
+if.end3:                                          ; preds = %if.then2, %if.end
+  ret void
+}
+
+define dso_local void @test_d(i32 noundef signext %flag) nounwind {
+; CHECK-LABEL: test_d:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    beqz $a0, .LBB3_2
+; CHECK-NEXT:  # %bb.1: # %if.then
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.d $a1, $a0
+; CHECK-NEXT:    ori $a1, $a1, 1
+; CHECK-NEXT:    iocsrwr.d $a1, $a0
+; CHECK-NEXT:  .LBB3_2: # %if.end
+; CHECK-NEXT:    ori $a0, $zero, 2
+; CHECK-NEXT:    iocsrrd.d $a0, $a0
+; CHECK-NEXT:    andi $a0, $a0, 1
+; CHECK-NEXT:    bnez $a0, .LBB3_4
+; CHECK-NEXT:  # %bb.3: # %if.then2
+; CHECK-NEXT:    b %plt(bug)
+; CHECK-NEXT:  .LBB3_4: # %if.end3
+; CHECK-NEXT:    ret
+entry:
+  %tobool.not = icmp eq i32 %flag, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = tail call i64 @llvm.loongarch.iocsrrd.d(i32 2)
+  %or = or i64 %0, 1
+  tail call void @llvm.loongarch.iocsrwr.d(i64 %or, i32 2)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  %1 = tail call i64 @llvm.loongarch.iocsrrd.d(i32 2)
+  %and = and i64 %1, 1
+  %tobool1.not = icmp eq i64 %and, 0
+  br i1 %tobool1.not, label %if.then2, label %if.end3
+
+if.then2:                                         ; preds = %if.end
+  tail call void @bug()
+  br label %if.end3
+
+if.end3:                                          ; preds = %if.then2, %if.end
+  ret void
+}

llvm/test/CodeGen/LoongArch/intrinsic-la64.ll

 entry:
   %0 = tail call i64 @llvm.loongarch.csrrd.d(i32 1)
   ret i64 %0
 }
 
 define void @csrrd_d_noret() {
 ; CHECK-LABEL: csrrd_d_noret:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    csrrd $a0, 1
 ; CHECK-NEXT:    ret
 entry:
   %0 = tail call i64 @llvm.loongarch.csrrd.d(i32 1)
   ret void
 }
 
 define i64 @csrwr_d(i64 %a) {
 ; CHECK-LABEL: csrwr_d:

 entry:
   %0 = tail call i64 @llvm.loongarch.iocsrrd.d(i32 %a)
   ret i64 %0
 }
 
 define void @iocsrrd_d_noret(i32 %a) {
 ; CHECK-LABEL: iocsrrd_d_noret:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    iocsrrd.d $a0, $a0
 ; CHECK-NEXT:    ret
 entry:
   %0 = tail call i64 @llvm.loongarch.iocsrrd.d(i32 %a)
   ret void
 }
 
 define void @iocsrwr_d(i64 %a, i32 signext %b) {
 ; CHECK-LABEL: iocsrwr_d:

llvm/test/CodeGen/LoongArch/intrinsic.ll

 entry:
   %0 = tail call i32 @llvm.loongarch.csrrd.w(i32 1)
   ret i32 %0
 }
 
 define void @csrrd_w_noret() {
 ; CHECK-LABEL: csrrd_w_noret:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    csrrd $a0, 1
 ; CHECK-NEXT:    ret
 entry:
   %0 = tail call i32 @llvm.loongarch.csrrd.w(i32 1)
   ret void
 }
 
 define i32 @csrwr_w(i32 signext %a) {
 ; CHECK-LABEL: csrwr_w:

 entry:
   %0 = tail call i32 @llvm.loongarch.iocsrrd.w(i32 %a)
   ret i32 %0
 }
 
 define void @iocsrrd_b_noret(i32 %a) {
 ; CHECK-LABEL: iocsrrd_b_noret:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    iocsrrd.b $a0, $a0
 ; CHECK-NEXT:    ret
 entry:
   %0 = tail call i32 @llvm.loongarch.iocsrrd.b(i32 %a)
   ret void
 }
 
 define void @iocsrrd_h_noret(i32 %a) {
 ; CHECK-LABEL: iocsrrd_h_noret:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    iocsrrd.h $a0, $a0
 ; CHECK-NEXT:    ret
 entry:
   %0 = tail call i32 @llvm.loongarch.iocsrrd.h(i32 %a)
   ret void
 }
 
 define void @iocsrrd_w_noret(i32 %a) {
 ; CHECK-LABEL: iocsrrd_w_noret:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    iocsrrd.w $a0, $a0
 ; CHECK-NEXT:    ret
 entry:
   %0 = tail call i32 @llvm.loongarch.iocsrrd.w(i32 %a)
   ret void
 }
 
 define void @iocsrwr_b(i32 %a, i32 %b) {
 ; CHECK-LABEL: iocsrwr_b: