[PATCH] D140988: [SystemZ] Implement lowering of GET_ROUNDING

[Tulio Magno Quites Machado Filho via Phabricator via llvm-commits]

tuliom marked 3 inline comments as done.
tuliom added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:9077
+                                          DAG.getConstant(3, dl, MVT::i32)),
+                              DAG.getConstant(3, dl, MVT::i32)),
+                  DAG.getConstant(1, dl, MVT::i32));
----------------
uweigand wrote:
> 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?
> 
Yes, it can.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:9085
+      DAG.getNode((VT.getSizeInBits() < 16 ? ISD::TRUNCATE : ISD::ZERO_EXTEND),
+                  dl, VT, RetVal);
+
----------------
uweigand wrote:
> 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?).
Oops. That's correct.
No, it does not get optimized away.


================
Comment at: llvm/test/CodeGen/SystemZ/flt-rounds.ll:28
+  %8 = call i32 @llvm.get.rounding()
+; CHECK: efpc %r{{[0-9]+}}
+  %9 = icmp ne i32 %7, %8
----------------
uweigand wrote:
> 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.
@uweigand I modified the test. Is this what you had in mind?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140988/new/

https://reviews.llvm.org/D140988