[PATCH] D23614: [PPC] Generate positive FP zero using xor insn instead of loading from constant area

Nemanja Ivanovic via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 17 13:14:21 PDT 2016


nemanjai added inline comments.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.h:746
@@ -745,2 +745,3 @@
 
+    bool isFPImmLegal(const APFloat &Imm, EVT VT) const override;
   private:
----------------
amehsan wrote:
> nemanjai wrote:
> > Perhaps a comment about what the use of this function is since this patch has none.
> The name is clear. Isn't it?
Oh, I see. This is used by the target-independent transformations.

================
Comment at: test/CodeGen/PowerPC/fast-isel-fcmp-nan.ll:5
@@ -4,3 +4,3 @@
 ; CHECK-LABEL: TestULT:
-; CHECK: mcrf
+; CHECK: xscmpudp
 ; CHECK: blr
----------------
amehsan wrote:
> nemanjai wrote:
> > I don't understand the changes to this test case. How is the VSX floating point comparison replacing the move of a field within the condition register and why? I don't really understand the intent of the original test either, but nonetheless checking for these instructions does not seem like the equivalent test.
> I was not able to understand the intention of this testcase. (That said, I didn't check git blame. There might be some info there).
> 
> The mcrf here, follows a fcmpu and copies BF 0 to BF7. the new VSX cmp insn, puts the result directly in BF 7. So that was the closest thing that I could have put here. I will check git blame, and if I found something useful to add, I will modify it. But with a testcase, whose intention is not known, I think this change should be fine.
Yeah ok. I agree. I think this is kind of a weird test case.


https://reviews.llvm.org/D23614





More information about the llvm-commits mailing list