[SystemZ] Implement lowering of GET_ROUNDING

[tuliom]

Summary

Add support for _FLT_ROUNDS_ in SystemZ.

[Kai]

llvm/test/CodeGen/SystemZ/flt-rounds.ll

2–4

The target triple is usually part of the command line.
This helps when testing other variations, e.g. s390x-zos.

[tuliom]

I'm fixing the issues pointed out by @Kai and I'm also fixing formatting issues.

[tuliom]

llvm/test/CodeGen/SystemZ/flt-rounds.ll

2–4

Fixed in the new revision.
Thanks!

[uweigand]

Thanks for working on this!

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

9077

This second AND seems redundant, can't this just be

DAG.getNode(ISD::XOR, dl, MVT::i32, CWD1,
                         DAG.getConstant(3, dl, MVT::i32)),

instead?

9085

Since we're doing the computation in i32, shouldn't this be a TRUNCATE for all sizes < 32 ? Also, if the size is exactly 32, we don't need either truncate or extend (not sure if the extend gets optimized away?).

llvm/test/CodeGen/SystemZ/flt-rounds.ll

28

This is actually a case where it probably would be a good idea to test for the full sequence, ideally by using an auto-generated test.

[tuliom]

Fix the issues pointed out by @uweigand.

[tuliom]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

9077

Yes, it can.

9085

Oops. That's correct.
No, it does not get optimized away.

llvm/test/CodeGen/SystemZ/flt-rounds.ll

28

@uweigand I modified the test. Is this what you had in mind?

[tuliom]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

9085

No, it does not get optimized away.

Let me correct myself: in a way, it does get optimized away.
But , AFAIU, the LLGFR is needed after RXSBG anyway.
You can see the generated code in the updated test.

[nikic]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

9083

nit: There is a DAG.getZExtOrTrunc() helper.

llvm/test/CodeGen/SystemZ/flt-rounds.ll

24

nit: You can add nounwind to avoid irrelevant cfi directives.

[uweigand]

With the changes pointed out by @nikic this looks good to me.

Optionally, there seems to be one more minor optimization opportunity. Looking at the generated code:

efpc %r0
nilf %r0, 3
lr %r1, %r0
xilf %r1, 2
rxsbg %r0, %r1, 33, 63, 63

I see that it should be possible to save one more instruction (and one register) by doing instead:

efpc %r0
nilf %r0, 3
rxsbg %r0, %r0, 33, 63, 63
xilf %r0, 1

I.e. re-associating the XORs to compute

CWD1 = efpc() & 3
CWD2 = (CWD1 ^ (CWD1 >> 1)) ^ 1

But , AFAIU, the LLGFR is needed after RXSBG anyway.

It's needed in your test case because the result is returned from the function, and function return values are implicitly extended in our ABI. If it were not directly returned, but used in some other way, the extension might not always be necessary.

[tuliom]

In D140988#4042746, @uweigand wrote:

CWD1 = efpc() & 3
CWD2 = (CWD1 ^ (CWD1 >> 1)) ^ 1

@uweigand This is not generating the right output.
Actually, the current revision here is already broken.
Originally, this patch had:

CWD2 = (~FPC ^ 0x3) >> 1

This can be rewritten as:

CWD2 = ((FPC >> 1) ^ 1 & 0x1)

But this is not going to save any instructions.

llvm/test/CodeGen/SystemZ/flt-rounds.ll

24

@nikic I'm afraid there is an issue between update_llc_test_checks.py and SystemZ.
Whenever nounwind is used, the CHECKs are removed.

[uweigand]

In D140988#4044864, @tuliom wrote:

In D140988#4042746, @uweigand wrote:

CWD1 = efpc() & 3
CWD2 = (CWD1 ^ (CWD1 >> 1)) ^ 1

@uweigand This is not generating the right output.

How so? Looks like this would produce:

CWD1  CWD1>>1  (CWD1 ^ (CWD1>>1))   (CWD1 ^ (CWD1>>1)) ^ 1
 00     00           00                    01
 01     00           01                    00
 10     01           11                    10
 11     01           10                    11

which seems exactly the transformation we want?

[tuliom]

Changes since revision 3:

1. Started to use getZExtOrTrunc.
2. Reverted the code for CWD2 to the same in revision 2 in order to fix an issue.
3. Added a comment describing how CWD2 is calculated.

[tuliom]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

9083

Done.

[tuliom]

@uweigand You're right.
I hadn't realized the changes to RetVal.
This revision includes that modification.

[uweigand]

; CHECK-NEXT:    efpc %r0
; CHECK-NEXT:    lr %r1, %r0
; CHECK-NEXT:    nilf %r1, 3
; CHECK-NEXT:    rxsbg %r1, %r0, 63, 63, 63
; CHECK-NEXT:    xilf %r1, 1

Huh. We still get the lr :-( That now seems an inefficiency in the RXSBG optimization pass, so it's not a problem with this patch, which now LGTM. Thanks again!

@jonpa maybe you can have a look into why it doesn't simply use the same register as source and destination of the RXSGB?

[tuliom]

In this revision, I fix the last missing parts from @nikic 's suggestions:

• I rebased the patches on top of commit 0f2c071fad60d7606ee1a05c71ab5e0510d5becc, allowing to use nounwind in the tests.
• I simplified the test_order's code.

[tuliom]

@uweigand I don't have commit access yet. If this new revision is still good, could you land this patch for me, please?
Please use “Tulio Magno Quites Machado Filho [e-mail]” to commit the change.

llvm/lib/Target/SystemZ/SystemZISelLowering.h

private:
   SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerSCALAR_TO_VECTOR(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerINSERT_VECTOR_ELT(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerEXTRACT_VECTOR_ELT(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerSIGN_EXTEND_VECTOR_INREG(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerZERO_EXTEND_VECTOR_INREG(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerShift(SDValue Op, SelectionDAG &DAG, unsigned ByScalar) const;
   SDValue lowerIS_FPCLASS(SDValue Op, SelectionDAG &DAG) const;
+  SDValue lowerGET_ROUNDING(SDValue Op, SelectionDAG &DAG) const;
 
   bool canTreatAsByteVector(EVT VT) const;
   SDValue combineExtract(const SDLoc &DL, EVT ElemVT, EVT VecVT, SDValue OrigOp,
                          unsigned Index, DAGCombinerInfo &DCI,
                          bool Force) const;
   SDValue combineTruncateExtract(const SDLoc &DL, EVT TruncVT, SDValue Op,
                                  DAGCombinerInfo &DCI) const;
   SDValue combineZERO_EXTEND(SDNode *N, DAGCombinerInfo &DCI) const;

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

SystemZTargetLowering::SystemZTargetLowering(const TargetMachine &TM,
   }
 
   // VASTART and VACOPY need to deal with the SystemZ-specific varargs
   // structure, but VAEND is a no-op.
   setOperationAction(ISD::VASTART, MVT::Other, Custom);
   setOperationAction(ISD::VACOPY,  MVT::Other, Custom);
   setOperationAction(ISD::VAEND,   MVT::Other, Expand);
 
+  setOperationAction(ISD::GET_ROUNDING, MVT::i32, Custom);
+
   // Codes for which we want to perform some z-specific combinations.
   setTargetDAGCombine({ISD::ZERO_EXTEND,
                        ISD::SIGN_EXTEND,
                        ISD::SIGN_EXTEND_INREG,
                        ISD::LOAD,
                        ISD::STORE,
                        ISD::VECTOR_SHUFFLE,
                        ISD::EXTRACT_VECTOR_ELT,
SDValue SystemZTargetLowering::LowerOperation(SDValue Op,
   case ISD::SHL:
     return lowerShift(Op, DAG, SystemZISD::VSHL_BY_SCALAR);
   case ISD::SRL:
     return lowerShift(Op, DAG, SystemZISD::VSRL_BY_SCALAR);
   case ISD::SRA:
     return lowerShift(Op, DAG, SystemZISD::VSRA_BY_SCALAR);
   case ISD::IS_FPCLASS:
     return lowerIS_FPCLASS(Op, DAG);
+  case ISD::GET_ROUNDING:
+    return lowerGET_ROUNDING(Op, DAG);
   default:
     llvm_unreachable("Unexpected node to lower");
   }
 }
 
 // Lower operations with invalid operand or result types (currently used
 // only for 128-bit integer types).
 void

 // This is only used by the isel schedulers, and is needed only to prevent
 // compiler from crashing when list-ilp is used.
 const TargetRegisterClass *
 SystemZTargetLowering::getRepRegClassFor(MVT VT) const {
   if (VT == MVT::Untyped)
     return &SystemZ::ADDR128BitRegClass;
   return TargetLowering::getRepRegClassFor(VT);
 }
+
+SDValue SystemZTargetLowering::lowerGET_ROUNDING(SDValue Op,
+                                                 SelectionDAG &DAG) const {
+  SDLoc dl(Op);
+  /*
+   The rounding method is in FPC Byte 3 bits 6-7, and has the following
+   settings:
+     00 Round to nearest
+     01 Round to 0
+     10 Round to +inf
+     11 Round to -inf
+
+  FLT_ROUNDS, on the other hand, expects the following:
+    -1 Undefined
+     0 Round to 0
+     1 Round to nearest
+     2 Round to +inf
+     3 Round to -inf
+  */
+
+  // Save FPC to register.
+  SDValue Chain = Op.getOperand(0);
+  SDValue EFPC(
+      DAG.getMachineNode(SystemZ::EFPC, dl, {MVT::i32, MVT::Other}, Chain), 0);
+  Chain = EFPC.getValue(1);
+
+  // Transform as necessary
+  SDValue CWD1 = DAG.getNode(ISD::AND, dl, MVT::i32, EFPC,
+                             DAG.getConstant(3, dl, MVT::i32));
+  SDValue CWD2 =
+      DAG.getNode(ISD::SRL, dl, MVT::i32,
+                  DAG.getNode(ISD::AND, dl, MVT::i32,
+                              DAG.getNode(ISD::XOR, dl, MVT::i32, EFPC,
+                                          DAG.getConstant(3, dl, MVT::i32)),
+                              DAG.getConstant(3, dl, MVT::i32)),
+                  DAG.getConstant(1, dl, MVT::i32));
+
+  SDValue RetVal = DAG.getNode(ISD::XOR, dl, MVT::i32, CWD1, CWD2);
+
+  EVT VT = Op.getValueType();
+  RetVal =
+      DAG.getNode((VT.getSizeInBits() < 16 ? ISD::TRUNCATE : ISD::ZERO_EXTEND),
+                  dl, VT, RetVal);
+
+  return DAG.getMergeValues({RetVal, Chain}, dl);
+}

llvm/test/CodeGen/SystemZ/flt-rounds.ll

+; RUN: llc -mtriple=s390x-linux-gnu -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=s390x-linux-gnu -verify-machineinstrs -O3 < %s | FileCheck %s
+
+@changed = dso_local global i32 0, align 4
+
+define dso_local signext i32 @test_flt_rounds() nounwind {
+; CHECK-LABEL: test_flt_rounds:
+  %1 = call i32 @llvm.get.rounding()
+; CHECK: efpc %r{{[0-9]+}}
+  ret i32 %1
+}
+
+declare i32 @llvm.get.rounding() nounwind
+
+define dso_local signext i32 @test_order(i32 noundef signext %0) nounwind {
+; CHECK-LABEL: test_order:
+  %2 = alloca i32, align 4
+  %3 = alloca i32, align 4
+  store i32 %0, ptr %2, align 4
+  %4 = call i32 @llvm.get.rounding()
+; CHECK: efpc %r{{[0-9]+}}
+  store i32 %4, ptr %3, align 4
+  %5 = load i32, ptr %2, align 4
+  %6 = call signext i32 @fesetround(i32 noundef signext %5) #3
+; CHECK: brasl %r{{[0-9]+}}, fesetround@PLT
+  %7 = load i32, ptr %3, align 4
+  %8 = call i32 @llvm.get.rounding()
+; CHECK: efpc %r{{[0-9]+}}
+  %9 = icmp ne i32 %7, %8
+  br i1 %9, label %10, label %11
+
+10:                                               ; preds = %1
+  store i32 1, ptr @changed, align 4
+  br label %11
+
+11:                                               ; preds = %10, %1
+  %12 = load i32, ptr %3, align 4
+  ret i32 %12
+}
+
+declare dso_local signext i32 @fesetround(i32 noundef signext) nounwind