[PATCH] D152714: [AArch64][Optimization]Solving the FCCMP issue

[Sam Tebbs via Phabricator via llvm-commits]

samtebbs added a comment.

Thank you for fixing up the other test changes, this is looking pretty good. I have some small changes to suggest.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16394-16413
+  SDLoc DL(N);
+  AArch64CC::CondCode AArch64CC;
+  SDValue Cmp,Cset;
+  SDValue SetCC = N->getOperand(0);
+if (SetCC.getOpcode() == ISD::SETCC && SetCC.getOperand(0).getValueType() == MVT::f32){
+
+  if (!DCI.isBeforeLegalize() &&
----------------
This doesn't quite match the [[ https://llvm.org/docs/CodingStandards.html#source-code-formatting | LLVM style guide ]]. You can try running `git-clang-format HEAD^` to fix it automatically. If that doesn't work, you can re-style the code manually to make it look like the rest of the file.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16394-16413
+  SDLoc DL(N);
+  AArch64CC::CondCode AArch64CC;
+  SDValue Cmp,Cset;
+  SDValue SetCC = N->getOperand(0);
+if (SetCC.getOpcode() == ISD::SETCC && SetCC.getOperand(0).getValueType() == MVT::f32){
+
+  if (!DCI.isBeforeLegalize() &&
----------------
samtebbs wrote:
> This doesn't quite match the [[ https://llvm.org/docs/CodingStandards.html#source-code-formatting | LLVM style guide ]]. You can try running `git-clang-format HEAD^` to fix it automatically. If that doesn't work, you can re-style the code manually to make it look like the rest of the file.
I think this code would be nice to have in a function, perhaps named something like `performANDSETCCCombine`? Some comments about the transformation it performs (turning `and(fcmp(a, b), fcmp(c, d))` into `fccmp(fcmp(a, b), c, d)`) would be good too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152714