[PATCH] D120647: [CGP] Sink compare through freeze

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:7904
+      if (TLI->useSoftFloat() && isa<FCmpInst>(CmpI))
+        return false;
+      // Check if the cmp node can be sunk, if so will will be duplicating the
----------------
Should we factor out this and multiple condition registers check as a separate function so that the check can be shared with `sinkCmpExpression`?


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:7905
+        return false;
+      // Check if the cmp node can be sunk, if so will will be duplicating the
+      // node anyway, fold it here to allow more transforms.
----------------
typo?


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:7911
+        return cast<Instruction>(U)->getParent() != CmpI->getParent();
+      });
+    };
----------------
Could you elaborate why this condition is necessary please?


================
Comment at: llvm/test/Transforms/CodeGenPrepare/X86/freeze-brcond.ll:55
 ; CHECK-NEXT:    [[C:%.*]] = icmp eq i32 0, 1
-; CHECK-NEXT:    br i1 [[C]], label [[A:%.*]], label [[B:%.*]]
+; CHECK-NEXT:    [[FR:%.*]] = freeze i1 [[C]]
+; CHECK-NEXT:    br i1 [[FR]], label [[A:%.*]], label [[B:%.*]]
----------------
This extra freeze here should be removed - would a simple constant-ness check using `Const0` and `Const1` be enough?
A slightly more expensive check would be `isGuaranteedNotToBeUndefOrPoison`, not sure it is fine for CodeGenPrepare to call it.


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

https://reviews.llvm.org/D120647