[PATCH] D121210: [X86] convertIntLogicToFPLogic - enable fp-logic on pre-AVX targets for supported fp predicates (PR34563)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:46826
+    SDValue Op10 = N10, Op11 = N11;
+    bool IsAlwaysSignaling0, IsAlwaysSignaling1;
+    unsigned SSECC0 = translateX86FSETCC(CC0, Op00, Op01, IsAlwaysSignaling0);
----------------
Make that a single "Unused" bool variable for readability? Or see next comment for an alternate change.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:46829
+    unsigned SSECC1 = translateX86FSETCC(CC1, Op10, Op11, IsAlwaysSignaling1);
+    if (SSECC0 >= 8 || SSECC1 >= 8)
+      return SDValue();
----------------
pengfei wrote:
> Move the comments on line 46820 here?
Instead of an ambiguous "8", can we spell this out to make the limitation clear?

IIUC, this is what we want to avoid:
    if (CC0 == ISD::SETONE || CC1 == ISD::SETONE ||
        CC0 == ISD::SETUEQ || CC1 == ISD::SETUEQ)

I see one other place in this file that uses this condition:
      // In the two cases not handled by SSE compare predicates (SETUEQ/SETONE),
      // emit two comparisons and a logic op to tie them together.

So we could create a small helper function to make both of these cases more obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121210